From 8f53d7bca45ad6d4c499265805ecd7725b346111 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Tue, 21 May 2024 13:06:22 -0700 Subject: [PATCH 01/19] AD-99 Updated ICollection to IList. Updated RaceTypeMetadata to be more descriptive as an int NumberOfChoices --- .../HelperTests/MerkleTreeTest.cs | 4 ++-- TrueVote.Api.Tests/ServiceTests/RaceTest.cs | 2 +- TrueVote.Api.Tests/TrueVote.Api.Tests.csproj | 2 +- TrueVote.Api/Models/ElectionModel.cs | 12 +++++----- TrueVote.Api/Models/RaceModel.cs | 19 ++++++++-------- TrueVote.Api/Startup.cs | 4 ++-- TrueVote.Api/TrueVote.Api.csproj | 22 +++++++++---------- 7 files changed, 32 insertions(+), 33 deletions(-) diff --git a/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs b/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs index a0f0057..b8a2c3b 100644 --- a/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs +++ b/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs @@ -70,8 +70,8 @@ public void CreatesHashFromBallotObject() Assert.NotNull(hash1); Assert.NotNull(hash2); Assert.Equal(hash1, hash2); - Assert.Equal("107", hash1[0].ToString()); - Assert.Equal("107", hash2[0].ToString()); + Assert.Equal("4", hash1[0].ToString()); + Assert.Equal("4", hash2[0].ToString()); } [Fact] diff --git a/TrueVote.Api.Tests/ServiceTests/RaceTest.cs b/TrueVote.Api.Tests/ServiceTests/RaceTest.cs index 3526dab..e901fbf 100644 --- a/TrueVote.Api.Tests/ServiceTests/RaceTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/RaceTest.cs @@ -24,7 +24,7 @@ public class FakeRaceModel public string RaceId { get; set; } = Guid.NewGuid().ToString(); public string Name { get; set; } = string.Empty; public RaceTypes RaceType { get; set; } - public ICollection Candidates { get; set; } = new List(); + public List Candidates { get; set; } = new List(); } public class RaceTest : TestHelper diff --git a/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj b/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj index 8f58831..ff81de6 100644 --- a/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj +++ b/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj @@ -17,7 +17,7 @@ all - + diff --git a/TrueVote.Api/Models/ElectionModel.cs b/TrueVote.Api/Models/ElectionModel.cs index 7ee79cc..7c29846 100644 --- a/TrueVote.Api/Models/ElectionModel.cs +++ b/TrueVote.Api/Models/ElectionModel.cs @@ -82,10 +82,10 @@ public class BaseElectionModel [Required] [Description("List of Races")] - [DataType("ICollection")] + [DataType("List")] [JsonPropertyName("Races")] [JsonProperty(nameof(Races), Required = Required.Always)] - public required ICollection Races { get; set; } = new List(); + public required List Races { get; set; } = new List(); } [ExcludeFromCodeCoverage] @@ -154,10 +154,10 @@ public class ElectionModel [Required] [Description("List of Races")] - [DataType("ICollection")] + [DataType("List")] [JsonPropertyName("Races")] [JsonProperty(nameof(Races), Required = Required.Always)] - public required ICollection Races { get; set; } = new List(); + public required List Races { get; set; } = new List(); } // Same as above model but without required properties @@ -221,10 +221,10 @@ public class ElectionModelResponse [Required] [Description("List of Races")] - [DataType("ICollection")] + [DataType("List")] [JsonPropertyName("Races")] [JsonProperty(nameof(Races), Required = Required.Always)] - public required ICollection Races { get; set; } = new List(); + public required List Races { get; set; } = new List(); } [ExcludeFromCodeCoverage] diff --git a/TrueVote.Api/Models/RaceModel.cs b/TrueVote.Api/Models/RaceModel.cs index 97cf935..bd8494d 100644 --- a/TrueVote.Api/Models/RaceModel.cs +++ b/TrueVote.Api/Models/RaceModel.cs @@ -107,12 +107,11 @@ public class RaceModel [JsonProperty(nameof(RaceTypeName), Required = Required.Default)] public string RaceTypeName => RaceType.ToString(); - [Description("Race Type Metadata")] - [MaxLength(2048)] - [DataType(DataType.Text)] - [JsonPropertyName("RaceTypeMetadata")] - [JsonProperty(nameof(RaceTypeMetadata), Required = Required.Default)] - public string? RaceTypeMetadata { get; set; } + [Description("Number of Choices")] + [DataType("integer")] + [JsonPropertyName("NumberOfChoices")] + [JsonProperty(nameof(NumberOfChoices), Required = Required.Default)] + public int? NumberOfChoices { get; set; } [Required] [Description("DateCreated")] @@ -123,10 +122,10 @@ public class RaceModel public required DateTime DateCreated { get; set; } = UtcNowProviderFactory.GetProvider().UtcNow; [Description("List of Candidates")] - [DataType("ICollection")] + [DataType("List")] [JsonPropertyName("Candidates")] [JsonProperty(nameof(Candidates), Required = Required.Default)] - public ICollection Candidates { get; set; } = new List(); + public List Candidates { get; set; } = new List(); } // Same as above model but without required properties @@ -177,10 +176,10 @@ public class RaceModelResponse [Required] [Description("List of Candidates")] - [DataType("ICollection")] + [DataType("List")] [JsonPropertyName("Candidates")] [JsonProperty(nameof(Candidates), Required = Required.Always)] - public required ICollection Candidates { get; set; } = new List(); + public required List Candidates { get; set; } = new List(); } [ExcludeFromCodeCoverage] diff --git a/TrueVote.Api/Startup.cs b/TrueVote.Api/Startup.cs index 63fb5c9..ddc7a19 100644 --- a/TrueVote.Api/Startup.cs +++ b/TrueVote.Api/Startup.cs @@ -262,7 +262,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) .HasConversion( v => JsonSerializer.Serialize(v, (JsonSerializerOptions) null), v => JsonSerializer.Deserialize>(v, (JsonSerializerOptions) null), - new ValueComparer>( + new ValueComparer>( (c1, c2) => c1.SequenceEqual(c2), c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())), c => c.ToList())); @@ -274,7 +274,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) .HasConversion( v => JsonSerializer.Serialize(v, (JsonSerializerOptions) null), v => JsonSerializer.Deserialize>(v, (JsonSerializerOptions) null), - new ValueComparer>( + new ValueComparer>( (c1, c2) => c1.SequenceEqual(c2), c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())), c => c.ToList())); diff --git a/TrueVote.Api/TrueVote.Api.csproj b/TrueVote.Api/TrueVote.Api.csproj index 3bdcce8..53df2c7 100644 --- a/TrueVote.Api/TrueVote.Api.csproj +++ b/TrueVote.Api/TrueVote.Api.csproj @@ -26,23 +26,23 @@ - - + + - - - - + + + + all runtime; build; native; contentfiles; analyzers; buildtransitive - + - - + + @@ -51,10 +51,10 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - + - + From fffbad189c0f47abee2814c32d42f9d0e246e32d Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Tue, 21 May 2024 14:48:25 -0700 Subject: [PATCH 02/19] AD-99 Renamed Validator to BallotValidator --- TrueVote.Api.Tests/Helpers/TestHelper.cs | 6 +++--- TrueVote.Api.Tests/ServiceTests/BallotTest.cs | 2 +- TrueVote.Api.Tests/ServiceTests/ValidatorTest.cs | 6 +++--- TrueVote.Api/Services/Ballot.cs | 4 ++-- TrueVote.Api/Services/{Validator.cs => BallotValidator.cs} | 6 +++--- TrueVote.Api/Startup.cs | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) rename TrueVote.Api/Services/{Validator.cs => BallotValidator.cs} (95%) diff --git a/TrueVote.Api.Tests/Helpers/TestHelper.cs b/TrueVote.Api.Tests/Helpers/TestHelper.cs index f3092bb..1ba933c 100644 --- a/TrueVote.Api.Tests/Helpers/TestHelper.cs +++ b/TrueVote.Api.Tests/Helpers/TestHelper.cs @@ -26,7 +26,7 @@ public class TestHelper protected readonly Ballot _ballotApi; protected readonly Race _raceApi; protected readonly Candidate _candidateApi; - protected readonly Validator _validatorApi; + protected readonly BallotValidator _validatorApi; protected readonly Timestamp _timestampApi; protected readonly Query _queryService; protected readonly MoqDataAccessor _moqDataAccessor; @@ -50,7 +50,7 @@ public TestHelper(ITestOutputHelper output) serviceCollection.TryAddScoped(); serviceCollection.TryAddSingleton(); serviceCollection.TryAddScoped(); - serviceCollection.TryAddScoped(); + serviceCollection.TryAddScoped(); serviceCollection.TryAddScoped(); serviceCollection.TryAddScoped(); var serviceProvider = serviceCollection.BuildServiceProvider(); @@ -78,7 +78,7 @@ public TestHelper(ITestOutputHelper output) _moqDataAccessor = new MoqDataAccessor(); _userApi = new User(_logHelper.Object, _moqDataAccessor.mockUserContext.Object, _mockServiceBus.Object, _mockJwtHandler.Object); _electionApi = new Election(_logHelper.Object, _moqDataAccessor.mockElectionContext.Object, _mockServiceBus.Object); - _validatorApi = new Validator(_logHelper.Object, _moqDataAccessor.mockBallotContext.Object, _mockOpenTimestampsClient.Object, _mockServiceBus.Object); + _validatorApi = new BallotValidator(_logHelper.Object, _moqDataAccessor.mockBallotContext.Object, _mockOpenTimestampsClient.Object, _mockServiceBus.Object); _ballotApi = new Ballot(_logHelper.Object, _moqDataAccessor.mockBallotContext.Object, _validatorApi, _mockServiceBus.Object); _raceApi = new Race(_logHelper.Object, _moqDataAccessor.mockRaceContext.Object, _mockServiceBus.Object); _candidateApi = new Candidate(_logHelper.Object, _moqDataAccessor.mockCandidateContext.Object, _mockServiceBus.Object); diff --git a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs index 9f5bc21..2efaa50 100644 --- a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs @@ -49,7 +49,7 @@ public async Task HandlesSubmitBallotHashingError() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; - var mockValidator = new Mock(); + var mockValidator = new Mock(); mockValidator.Setup(m => m.HashBallotAsync(It.IsAny())).Throws(new Exception("Hash Ballot Exception")); var ballotApi = new Ballot(_logHelper.Object, _moqDataAccessor.mockBallotContext.Object, mockValidator.Object, _mockServiceBus.Object); diff --git a/TrueVote.Api.Tests/ServiceTests/ValidatorTest.cs b/TrueVote.Api.Tests/ServiceTests/ValidatorTest.cs index 42e6cfd..f1a1b38 100644 --- a/TrueVote.Api.Tests/ServiceTests/ValidatorTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/ValidatorTest.cs @@ -54,7 +54,7 @@ public async Task HashesBallotThrowsStoreTimestampException() mockBallotContext.Setup(m => m.BallotHashes).Returns(MockBallotHashSet.Object); mockBallotContext.Setup(m => m.EnsureCreatedAsync()).Throws(new Exception("Storing data exception")); - var validatorApi = new Validator(_logHelper.Object, mockBallotContext.Object, _mockOpenTimestampsClient.Object, _mockServiceBus.Object); + var validatorApi = new BallotValidator(_logHelper.Object, mockBallotContext.Object, _mockOpenTimestampsClient.Object, _mockServiceBus.Object); try { @@ -108,7 +108,7 @@ public async Task StoreBallotHashAsyncThrowsException() mockBallotHashContext.Setup(m => m.BallotHashes).Returns(MockBallotHashSet.Object); mockBallotHashContext.Setup(m => m.EnsureCreatedAsync()).Throws(new Exception("Storing data exception")); - var validatorApi = new Validator(_logHelper.Object, mockBallotHashContext.Object, _mockOpenTimestampsClient.Object, _mockServiceBus.Object); + var validatorApi = new BallotValidator(_logHelper.Object, mockBallotHashContext.Object, _mockOpenTimestampsClient.Object, _mockServiceBus.Object); try { @@ -132,7 +132,7 @@ public async Task StoreTimestampAsyncThrowsException() mockTimestampContext.Setup(m => m.Timestamps).Returns(MockTimestampSet.Object); mockTimestampContext.Setup(m => m.EnsureCreatedAsync()).Throws(new Exception("Storing data exception")); - var validatorApi = new Validator(_logHelper.Object, mockTimestampContext.Object, _mockOpenTimestampsClient.Object, _mockServiceBus.Object); + var validatorApi = new BallotValidator(_logHelper.Object, mockTimestampContext.Object, _mockOpenTimestampsClient.Object, _mockServiceBus.Object); try { diff --git a/TrueVote.Api/Services/Ballot.cs b/TrueVote.Api/Services/Ballot.cs index d1e3fcc..5bb5012 100644 --- a/TrueVote.Api/Services/Ballot.cs +++ b/TrueVote.Api/Services/Ballot.cs @@ -20,10 +20,10 @@ public class Ballot : ControllerBase { private readonly ILogger _log; private readonly ITrueVoteDbContext _trueVoteDbContext; - private readonly IValidator _validator; + private readonly IBallotValidator _validator; private readonly IServiceBus _serviceBus; - public Ballot(ILogger log, ITrueVoteDbContext trueVoteDbContext, IValidator validator, IServiceBus serviceBus) + public Ballot(ILogger log, ITrueVoteDbContext trueVoteDbContext, IBallotValidator validator, IServiceBus serviceBus) { _log = log; _trueVoteDbContext = trueVoteDbContext; diff --git a/TrueVote.Api/Services/Validator.cs b/TrueVote.Api/Services/BallotValidator.cs similarity index 95% rename from TrueVote.Api/Services/Validator.cs rename to TrueVote.Api/Services/BallotValidator.cs index fa674af..9c00041 100644 --- a/TrueVote.Api/Services/Validator.cs +++ b/TrueVote.Api/Services/BallotValidator.cs @@ -6,7 +6,7 @@ namespace TrueVote.Api.Services { - public interface IValidator + public interface IBallotValidator { Task HashBallotAsync(BallotModel ballot); Task HashBallotsAsync(); @@ -14,14 +14,14 @@ public interface IValidator Task StoreBallotHashAsync(BallotHashModel ballotHashModel); } - public class Validator : IValidator + public class BallotValidator : IBallotValidator { private readonly ITrueVoteDbContext _trueVoteDbContext; private readonly IOpenTimestampsClient _openTimestampsClient; private readonly IServiceBus _serviceBus; private readonly ILogger _log; - public Validator(ILogger log, ITrueVoteDbContext trueVoteDbContext, IOpenTimestampsClient openTimestampsClient, IServiceBus serviceBus) + public BallotValidator(ILogger log, ITrueVoteDbContext trueVoteDbContext, IOpenTimestampsClient openTimestampsClient, IServiceBus serviceBus) { _trueVoteDbContext = trueVoteDbContext; _openTimestampsClient = openTimestampsClient; diff --git a/TrueVote.Api/Startup.cs b/TrueVote.Api/Startup.cs index ddc7a19..c7c2c2e 100644 --- a/TrueVote.Api/Startup.cs +++ b/TrueVote.Api/Startup.cs @@ -144,7 +144,7 @@ public void ConfigureServices(IServiceCollection services) var uri = provider.GetRequiredService(); client.BaseAddress = uri; }); - services.TryAddScoped(); + services.TryAddScoped(); services.AddLogging(); services.AddSingleton(typeof(ILogger), typeof(Logger)); services.AddGraphQLServer().AddQueryType(); From 52b1ecd784b805824a229a14243edf0bf381113c Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Tue, 21 May 2024 15:20:47 -0700 Subject: [PATCH 03/19] AD-99 Added CustomValidator helper. Added Validator to ensure NumberOfChoices <= Race.Candidates.Count. --- TrueVote.Api/Helpers/CustomValidators.cs | 52 ++++++++++++++++++++++++ TrueVote.Api/Models/RaceModel.cs | 4 +- 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 TrueVote.Api/Helpers/CustomValidators.cs diff --git a/TrueVote.Api/Helpers/CustomValidators.cs b/TrueVote.Api/Helpers/CustomValidators.cs new file mode 100644 index 0000000..b1aed01 --- /dev/null +++ b/TrueVote.Api/Helpers/CustomValidators.cs @@ -0,0 +1,52 @@ +#pragma warning disable IDE0046 // Convert to conditional expression +using System.Collections; +using System.ComponentModel.DataAnnotations; +using System.Diagnostics.CodeAnalysis; + +namespace TrueVote.Api.Helpers +{ + [ExcludeFromCodeCoverage] + public class NumberOfChoicesValidatorAttribute : ValidationAttribute + { + private readonly string _propertyName; + + public NumberOfChoicesValidatorAttribute(string propertyName) + { + _propertyName = propertyName; + } + + protected override ValidationResult IsValid(object value, ValidationContext validationContext) + { + // Get the property info of the property + var propertyInfo = validationContext.ObjectType.GetProperty(_propertyName); + + if (propertyInfo == null) + { + return new ValidationResult($"Property '{_propertyName}' not found."); + } + + // Get the value of the property + + if (propertyInfo.GetValue(validationContext.ObjectInstance) is not IEnumerable propertyValue) + { + return new ValidationResult($"Property '{_propertyName}' is not a valid collection."); + } + + // Calculate the count of the collection + var count = 0; + foreach (var _ in propertyValue) + { + count++; + } + + var numberOfChoices = value as int?; + if (numberOfChoices.HasValue && numberOfChoices.Value > count) + { + return new ValidationResult($"Number of choices cannot exceed the number of {_propertyName}. NumberOfChoices: {numberOfChoices}, {_propertyName}: {count}"); + } + + return ValidationResult.Success; + } + } +} +#pragma warning restore IDE0046 // Convert to conditional expression diff --git a/TrueVote.Api/Models/RaceModel.cs b/TrueVote.Api/Models/RaceModel.cs index bd8494d..ab59ca8 100644 --- a/TrueVote.Api/Models/RaceModel.cs +++ b/TrueVote.Api/Models/RaceModel.cs @@ -109,8 +109,10 @@ public class RaceModel [Description("Number of Choices")] [DataType("integer")] - [JsonPropertyName("NumberOfChoices")] + [Range(0, int.MaxValue)] + [JsonPropertyName("NumberOfChoices")] [JsonProperty(nameof(NumberOfChoices), Required = Required.Default)] + [NumberOfChoicesValidator("Candidates")] public int? NumberOfChoices { get; set; } [Required] From c9b41ef117bbbe3781b7963ac8607223855b58cc Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Tue, 21 May 2024 17:05:22 -0700 Subject: [PATCH 04/19] AD-99 Optimized CustomValidator --- TrueVote.Api/Helpers/CustomValidators.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/TrueVote.Api/Helpers/CustomValidators.cs b/TrueVote.Api/Helpers/CustomValidators.cs index b1aed01..4483df3 100644 --- a/TrueVote.Api/Helpers/CustomValidators.cs +++ b/TrueVote.Api/Helpers/CustomValidators.cs @@ -26,23 +26,18 @@ protected override ValidationResult IsValid(object value, ValidationContext vali } // Get the value of the property - if (propertyInfo.GetValue(validationContext.ObjectInstance) is not IEnumerable propertyValue) { return new ValidationResult($"Property '{_propertyName}' is not a valid collection."); } - // Calculate the count of the collection - var count = 0; - foreach (var _ in propertyValue) - { - count++; - } + // Calculate the count of the collection using LINQ + var count = propertyValue.Cast().Count(); var numberOfChoices = value as int?; if (numberOfChoices.HasValue && numberOfChoices.Value > count) { - return new ValidationResult($"Number of choices cannot exceed the number of {_propertyName}. NumberOfChoices: {numberOfChoices}, {_propertyName}: {count}"); + return new ValidationResult($"Number of Choices cannot exceed the number of items in '{_propertyName}'. NumberOfChoices: {numberOfChoices}, Count: {count}"); } return ValidationResult.Success; From 4c334d586e689a8d4cd0e22f20f18bc1954490e5 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Wed, 22 May 2024 13:20:19 -0700 Subject: [PATCH 05/19] AD-99 Added Validator test to traverse through models and enforce all the data annotations for unit tests. Improved coverage. --- .../HelperTests/CustomValidatorTest.cs | 141 ++++++++++++++++++ .../Helpers/ValidationHelper.cs | 71 +++++++++ TrueVote.Api.Tests/ServiceTests/BallotTest.cs | 31 ++++ TrueVote.Api/Helpers/CustomValidators.cs | 15 +- TrueVote.Api/Models/RaceModel.cs | 2 +- 5 files changed, 251 insertions(+), 9 deletions(-) create mode 100644 TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs create mode 100644 TrueVote.Api.Tests/Helpers/ValidationHelper.cs diff --git a/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs b/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs new file mode 100644 index 0000000..6c83e78 --- /dev/null +++ b/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs @@ -0,0 +1,141 @@ +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.ComponentModel; +using Xunit; +using TrueVote.Api.Tests.Helpers; +using TrueVote.Api.Helpers; +using Newtonsoft.Json; +using System.Linq; + +namespace TrueVote.Api.Tests.HelperTests +{ + public class NameModel + { + [Required] + [Description("Name")] + [MaxLength(2048)] + [DataType(DataType.Text)] + public required string Name { get; set; } = string.Empty; + } + + public class TestModel1 + { + [Description("Number of Choices")] + [DataType("integer")] + [Range(0, int.MaxValue)] + [NumberOfChoicesValidator(nameof(NameModel))] + public int? NumberOfChoices { get; set; } + + [Description("List of Names")] + [DataType("List")] + public List NameModel { get; set; } = new List(); + } + + public class TestModel2 + { + [Description("Number of Choices")] + [DataType("integer")] + [Range(0, int.MaxValue)] + [NumberOfChoicesValidator("")] + public int? NumberOfChoices { get; set; } + + [Description("List of Names")] + [DataType("List")] + public List NameModel { get; set; } = new List(); + } + + public class TestModel3 + { + [Description("Number of Choices")] + [DataType("integer")] + [Range(0, int.MaxValue)] + [NumberOfChoicesValidator(nameof(Name))] + public int? NumberOfChoices { get; set; } + + [Description("Name")] + [DataType(DataType.Text)] + public string Name { get; set; } = string.Empty; + } + + public class TestModel5 + { + [Description("Number of Choices")] + [DataType("integer")] + [Range(0, int.MaxValue)] + [NumberOfChoicesValidator(nameof(NameModel))] + public int? NumberOfChoices { get; set; } + + [Description("List of Names")] + [DataType("List")] + public List NameModel { get; set; } = new List(); + } + + public class CustomValidatorTest + { + [Fact] + public void TestsNumberOfChoicesSucceeds() + { + var testModel = new TestModel1 { NumberOfChoices = 1 }; + testModel.NameModel.Add(new NameModel { Name = "name1 "}); + testModel.NameModel.Add(new NameModel { Name = "name2 " }); + testModel.NameModel.Add(new NameModel { Name = "name3 " }); + + var validationResults = ValidationHelper.Validate(testModel); + Assert.Empty(validationResults); + } + + [Fact] + public void TestsNumberOfChoicesFailsWhenTooHigh() + { + var testModel = new TestModel1 { NumberOfChoices = 5 }; + testModel.NameModel.Add(new NameModel { Name = "name1 " }); + testModel.NameModel.Add(new NameModel { Name = "name2 " }); + testModel.NameModel.Add(new NameModel { Name = "name3 " }); + + var validationResults = ValidationHelper.Validate(testModel); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("NumberOfChoices cannot exceed the", validationResults[0].ErrorMessage); + Assert.Equal("NumberOfChoices", validationResults[0].MemberNames.First()); + } + + [Fact] + public void TestsNumberOfChoicesFailsWhenPropertyNotFound() + { + var testModel = new TestModel2 { NumberOfChoices = 1 }; + testModel.NameModel.Add(new NameModel { Name = "name1 " }); + testModel.NameModel.Add(new NameModel { Name = "name2 " }); + testModel.NameModel.Add(new NameModel { Name = "name3 " }); + + var validationResults = ValidationHelper.Validate(testModel); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("Property not found", validationResults[0].ErrorMessage); + Assert.Equal("NumberOfChoices", validationResults[0].MemberNames.First()); + } + + [Fact] + public void TestsNumberOfChoicesFailsWhenPropertyToCheckIsInvalid() + { + var testModel = new TestModel3 { NumberOfChoices = 1 }; + + var validationResults = ValidationHelper.Validate(testModel); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("is not a valid collection", validationResults[0].ErrorMessage); + Assert.Equal("NumberOfChoices", validationResults[0].MemberNames.First()); + } + + [Fact] + public void TestsNumberOfChoicesSucceedsWhenNumberOfChoicesIsUnset() + { + var testModel = new TestModel5(); + + var validationResults = ValidationHelper.Validate(testModel); + Assert.Empty(validationResults); + } + } +} diff --git a/TrueVote.Api.Tests/Helpers/ValidationHelper.cs b/TrueVote.Api.Tests/Helpers/ValidationHelper.cs new file mode 100644 index 0000000..92c6d2e --- /dev/null +++ b/TrueVote.Api.Tests/Helpers/ValidationHelper.cs @@ -0,0 +1,71 @@ +using System.Collections; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; + +namespace TrueVote.Api.Tests.Helpers +{ + public static class ValidationHelper + { + public static IList Validate(object model) + { + var validationResults = new List(); + ValidateRecursive(model, validationResults); + return validationResults; + } + + private static void ValidateRecursive(object instance, IList validationResults) + { + if (instance == null) + return; + + var context = new ValidationContext(instance); + var results = new List(); + + Validator.TryValidateObject(instance, context, results, true); + foreach (var validationResult in results) + { + validationResults.Add(validationResult); + } + + var properties = instance.GetType().GetProperties(); + foreach (var property in properties) + { + var propertyValue = property.GetValue(instance); + if (propertyValue == null) + continue; + + // Check if property is a collection or complex type + if (IsCollection(propertyValue) || IsComplexType(propertyValue)) + { + if (IsCollection(propertyValue)) + { + if (propertyValue is IEnumerable enumerable) + { + foreach (var item in enumerable) + { + if (item != null) + { + ValidateRecursive(item, validationResults); + } + } + } + } + else + { + ValidateRecursive(propertyValue, validationResults); + } + } + } + } + + private static bool IsCollection(object obj) + { + return obj is IEnumerable and not string; + } + + private static bool IsComplexType(object obj) + { + return !obj.GetType().IsValueType && obj.GetType() != typeof(string); + } + } +} diff --git a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs index 2efaa50..413cc80 100644 --- a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs @@ -5,6 +5,7 @@ using Moq; using System; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using TrueVote.Api.Models; using TrueVote.Api.Services; @@ -24,6 +25,8 @@ public BallotTest(ITestOutputHelper output) : base(output) public async Task SubmitsBallot() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; + var validationResults = ValidationHelper.Validate(baseBallotObj); + Assert.Empty(validationResults); var ret = await _ballotApi.SubmitBallot(baseBallotObj); Assert.NotNull(ret); @@ -44,6 +47,34 @@ public async Task SubmitsBallot() _logHelper.Verify(LogLevel.Debug, Times.Exactly(2)); } + [Fact] + public void SubmitsBallotWithInvalidNumberOfChoices() + { + var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; + baseBallotObj.Election.Races[0].NumberOfChoices = -1; + + var validationResults = ValidationHelper.Validate(baseBallotObj); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("NumberOfChoices must be between 0", validationResults[0].ErrorMessage); + Assert.Equal("NumberOfChoices", validationResults[0].MemberNames.First()); + } + + [Fact] + public void SubmitsBallotWithAboveCandidateCountNumberOfChoices() + { + var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; + baseBallotObj.Election.Races[0].NumberOfChoices = 20; + + var validationResults = ValidationHelper.Validate(baseBallotObj); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("NumberOfChoices cannot exceed the", validationResults[0].ErrorMessage); + Assert.Equal("NumberOfChoices", validationResults[0].MemberNames.First()); + } + [Fact] public async Task HandlesSubmitBallotHashingError() { diff --git a/TrueVote.Api/Helpers/CustomValidators.cs b/TrueVote.Api/Helpers/CustomValidators.cs index 4483df3..2c44117 100644 --- a/TrueVote.Api/Helpers/CustomValidators.cs +++ b/TrueVote.Api/Helpers/CustomValidators.cs @@ -1,11 +1,9 @@ #pragma warning disable IDE0046 // Convert to conditional expression using System.Collections; using System.ComponentModel.DataAnnotations; -using System.Diagnostics.CodeAnalysis; namespace TrueVote.Api.Helpers { - [ExcludeFromCodeCoverage] public class NumberOfChoicesValidatorAttribute : ValidationAttribute { private readonly string _propertyName; @@ -22,22 +20,23 @@ protected override ValidationResult IsValid(object value, ValidationContext vali if (propertyInfo == null) { - return new ValidationResult($"Property '{_propertyName}' not found."); + return new ValidationResult($"Property not found.", [validationContext.MemberName]); } // Get the value of the property - if (propertyInfo.GetValue(validationContext.ObjectInstance) is not IEnumerable propertyValue) + var propertyValue = propertyInfo.GetValue(validationContext.ObjectInstance); + if (propertyValue is not IEnumerable or string or int or long or DateTime) { - return new ValidationResult($"Property '{_propertyName}' is not a valid collection."); + return new ValidationResult($"Property '{_propertyName}' is not a valid collection.", [validationContext.MemberName]); } - // Calculate the count of the collection using LINQ - var count = propertyValue.Cast().Count(); + // Calculate the count of the collection + var count = ((IEnumerable) propertyValue).Cast().Count(); var numberOfChoices = value as int?; if (numberOfChoices.HasValue && numberOfChoices.Value > count) { - return new ValidationResult($"Number of Choices cannot exceed the number of items in '{_propertyName}'. NumberOfChoices: {numberOfChoices}, Count: {count}"); + return new ValidationResult($"NumberOfChoices cannot exceed the number of items in '{_propertyName}'. NumberOfChoices: {numberOfChoices}, Count: {count}", [validationContext.MemberName]); } return ValidationResult.Success; diff --git a/TrueVote.Api/Models/RaceModel.cs b/TrueVote.Api/Models/RaceModel.cs index ab59ca8..62c0006 100644 --- a/TrueVote.Api/Models/RaceModel.cs +++ b/TrueVote.Api/Models/RaceModel.cs @@ -112,7 +112,7 @@ public class RaceModel [Range(0, int.MaxValue)] [JsonPropertyName("NumberOfChoices")] [JsonProperty(nameof(NumberOfChoices), Required = Required.Default)] - [NumberOfChoicesValidator("Candidates")] + [NumberOfChoicesValidator(nameof(Candidates))] public int? NumberOfChoices { get; set; } [Required] From 374cbacfe1d4017f092c84a99307c885ffcb71d6 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Wed, 29 May 2024 13:34:32 -0700 Subject: [PATCH 06/19] Updated Packages --- TrueVote.Api.Tests/TrueVote.Api.Tests.csproj | 8 ++++---- TrueVote.Api/TrueVote.Api.csproj | 18 +++++++++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj b/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj index ff81de6..b1bcd6c 100644 --- a/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj +++ b/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj @@ -18,12 +18,12 @@ - + - - - + + + runtime; build; native; contentfiles; analyzers; buildtransitive all diff --git a/TrueVote.Api/TrueVote.Api.csproj b/TrueVote.Api/TrueVote.Api.csproj index 53df2c7..6fa7473 100644 --- a/TrueVote.Api/TrueVote.Api.csproj +++ b/TrueVote.Api/TrueVote.Api.csproj @@ -26,14 +26,14 @@ - - + + - - - - + + + + all runtime; build; native; contentfiles; analyzers; buildtransitive @@ -41,8 +41,8 @@ - - + + @@ -54,7 +54,7 @@ - + From 189cdd6ce9ff14c39a2c1e35e19f160f10548b11 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Wed, 29 May 2024 13:53:29 -0700 Subject: [PATCH 07/19] AD-99 Improved property helper. Added property validation to all tests. Removed [ExcludeFromCodeCoverage] on all models now that they're properly testable with this new pattern. --- .../Helpers/ValidationHelper.cs | 17 +++- TrueVote.Api.Tests/ServiceTests/BallotTest.cs | 12 +++ .../ServiceTests/CandidateTest.cs | 8 ++ .../ServiceTests/ElectionTest.cs | 16 ++++ .../ServiceTests/Error500Test.cs | 4 + TrueVote.Api.Tests/ServiceTests/RaceTest.cs | 16 ++++ TrueVote.Api.Tests/ServiceTests/UserTest.cs | 69 +++++++++++++-- TrueVote.Api/Models/BallotModel.cs | 10 --- TrueVote.Api/Models/CandidateModel.cs | 6 -- TrueVote.Api/Models/ElectionModel.cs | 85 +------------------ TrueVote.Api/Models/FeedbackModel.cs | 2 - TrueVote.Api/Models/HelperModel.cs | 3 - TrueVote.Api/Models/RaceModel.cs | 67 --------------- TrueVote.Api/Models/StatusModel.cs | 3 - TrueVote.Api/Models/TimestampModel.cs | 3 - TrueVote.Api/Models/UserModel.cs | 9 -- 16 files changed, 133 insertions(+), 197 deletions(-) diff --git a/TrueVote.Api.Tests/Helpers/ValidationHelper.cs b/TrueVote.Api.Tests/Helpers/ValidationHelper.cs index 92c6d2e..9fa4c3b 100644 --- a/TrueVote.Api.Tests/Helpers/ValidationHelper.cs +++ b/TrueVote.Api.Tests/Helpers/ValidationHelper.cs @@ -1,3 +1,4 @@ +using System; using System.Collections; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; @@ -30,7 +31,19 @@ private static void ValidateRecursive(object instance, IList v var properties = instance.GetType().GetProperties(); foreach (var property in properties) { - var propertyValue = property.GetValue(instance); + object propertyValue = null; + + try + { + propertyValue = property.GetValue(instance); + } + catch (Exception ex) + { + // Log or handle the exception if necessary + Console.WriteLine($"Exception when getting value of property '{property.Name}': {ex.Message}"); + continue; // Skip this property and move to the next one + } + if (propertyValue == null) continue; @@ -60,7 +73,7 @@ private static void ValidateRecursive(object instance, IList v private static bool IsCollection(object obj) { - return obj is IEnumerable and not string; + return obj is IEnumerable && obj.GetType() != typeof(string); } private static bool IsComplexType(object obj) diff --git a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs index 413cc80..36fac92 100644 --- a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs @@ -79,6 +79,8 @@ public void SubmitsBallotWithAboveCandidateCountNumberOfChoices() public async Task HandlesSubmitBallotHashingError() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; + var validationResults = ValidationHelper.Validate(baseBallotObj); + Assert.Empty(validationResults); var mockValidator = new Mock(); mockValidator.Setup(m => m.HashBallotAsync(It.IsAny())).Throws(new Exception("Hash Ballot Exception")); @@ -100,6 +102,8 @@ public async Task HandlesSubmitBallotHashingError() public async Task FindsBallot() { var findBallotObj = new FindBallotModel { BallotId = "ballotid3" }; + var validationResults = ValidationHelper.Validate(findBallotObj); + Assert.Empty(validationResults); var ret = await _ballotApi.BallotFind(findBallotObj); Assert.NotNull(ret); @@ -118,6 +122,8 @@ public async Task FindsBallot() public async Task HandlesUnfoundBallot() { var findBallotObj = new FindBallotModel { BallotId = "not going to find anything" }; + var validationResults = ValidationHelper.Validate(findBallotObj); + Assert.Empty(validationResults); var ret = await _ballotApi.BallotFind(findBallotObj); Assert.NotNull(ret); @@ -131,6 +137,8 @@ public async Task HandlesUnfoundBallot() public async Task CountsBallots() { var countBallotsObj = new CountBallotModel { DateCreatedStart = new DateTime(2022, 01, 01), DateCreatedEnd = new DateTime(2033, 12, 31) }; + var validationResults = ValidationHelper.Validate(countBallotsObj); + Assert.Empty(validationResults); var ret = await _ballotApi.BallotCount(countBallotsObj); Assert.NotNull(ret); @@ -148,6 +156,8 @@ public async Task CountsBallots() public async Task FindsBallotHash() { var findBallotHashObj = new FindBallotHashModel { BallotId = "ballotid1" }; + var validationResults = ValidationHelper.Validate(findBallotHashObj); + Assert.Empty(validationResults); var ret = await _ballotApi.BallotHashFind(findBallotHashObj); Assert.NotNull(ret); @@ -166,6 +176,8 @@ public async Task FindsBallotHash() public async Task HandlesUnfoundBallotHash() { var findBallotHashObj = new FindBallotHashModel { BallotId = "not going to find anything" }; + var validationResults = ValidationHelper.Validate(findBallotHashObj); + Assert.Empty(validationResults); var ret = await _ballotApi.BallotHashFind(findBallotHashObj); Assert.NotNull(ret); diff --git a/TrueVote.Api.Tests/ServiceTests/CandidateTest.cs b/TrueVote.Api.Tests/ServiceTests/CandidateTest.cs index e2c8bcb..f511428 100644 --- a/TrueVote.Api.Tests/ServiceTests/CandidateTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/CandidateTest.cs @@ -28,6 +28,8 @@ public CandidateTest(ITestOutputHelper output) : base(output) public async Task LogsMessages() { var baseCandidateObj = new BaseCandidateModel { Name = "John Smith", PartyAffiliation = "Republican" }; + var validationResults = ValidationHelper.Validate(baseCandidateObj); + Assert.Empty(validationResults); await _candidateApi.CreateCandidate(baseCandidateObj); @@ -39,6 +41,8 @@ public async Task LogsMessages() public async Task AddsCandidate() { var baseCandidateObj = new BaseCandidateModel { Name = "John Smith", PartyAffiliation = "Republican" }; + var validationResults = ValidationHelper.Validate(baseCandidateObj); + Assert.Empty(validationResults); var ret = await _candidateApi.CreateCandidate(baseCandidateObj); Assert.NotNull(ret); @@ -67,6 +71,8 @@ public async Task AddsCandidate() public async Task FindsCandidate() { var findCandidateObj = new FindCandidateModel { Name = "J" }; + var validationResults = ValidationHelper.Validate(findCandidateObj); + Assert.Empty(validationResults); var ret = await _candidateApi.CandidateFind(findCandidateObj); Assert.NotNull(ret); @@ -86,6 +92,8 @@ public async Task FindsCandidate() public async Task HandlesUnfoundCandidate() { var findCandidateObj = new FindCandidateModel { Name = "not going to find anything" }; + var validationResults = ValidationHelper.Validate(findCandidateObj); + Assert.Empty(validationResults); var ret = await _candidateApi.CandidateFind(findCandidateObj); Assert.NotNull(ret); diff --git a/TrueVote.Api.Tests/ServiceTests/ElectionTest.cs b/TrueVote.Api.Tests/ServiceTests/ElectionTest.cs index f4c0947..086cad6 100644 --- a/TrueVote.Api.Tests/ServiceTests/ElectionTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/ElectionTest.cs @@ -31,6 +31,8 @@ public ElectionTest(ITestOutputHelper output) : base(output) public async Task LogsMessages() { var baseElectionObj = new BaseElectionModel { Name = "California State", StartDate = DateTime.Now, EndDate = DateTime.Now.AddDays(30), Description = "desc", HeaderImageUrl = "url", Races = [] }; + var validationResults = ValidationHelper.Validate(baseElectionObj); + Assert.Empty(validationResults); await _electionApi.CreateElection(baseElectionObj); @@ -42,6 +44,8 @@ public async Task LogsMessages() public async Task AddsElection() { var baseElectionObj = new BaseElectionModel { Name = "California State", StartDate = DateTime.Now, EndDate = DateTime.Now.AddDays(30), Description = "desc", HeaderImageUrl = "url", Races = [] }; + var validationResults = ValidationHelper.Validate(baseElectionObj); + Assert.Empty(validationResults); var ret = await _electionApi.CreateElection(baseElectionObj); Assert.NotNull(ret); @@ -70,6 +74,8 @@ public async Task AddsElection() public async Task FindsElection() { var findElectionObj = new FindElectionModel { Name = "County" }; + var validationResults = ValidationHelper.Validate(findElectionObj); + Assert.Empty(validationResults); var ret = await _electionApi.ElectionFind(findElectionObj); Assert.NotNull(ret); @@ -88,6 +94,8 @@ public async Task FindsElection() public async Task HandlesUnfoundElection() { var findElectionObj = new FindElectionModel { Name = "not going to find anything" }; + var validationResults = ValidationHelper.Validate(findElectionObj); + Assert.Empty(validationResults); var ret = await _electionApi.ElectionFind(findElectionObj); Assert.NotNull(ret); @@ -101,6 +109,8 @@ public async Task HandlesUnfoundElection() public async Task AddsRacesToElection() { var addRacesObj = new AddRacesModel { ElectionId = MoqData.MockElectionData[2].ElectionId, RaceIds = new List { MoqData.MockRaceData[0].RaceId, MoqData.MockRaceData[1].RaceId, MoqData.MockRaceData[2].RaceId } }; + var validationResults = ValidationHelper.Validate(addRacesObj); + Assert.Empty(validationResults); var ret = await _electionApi.AddRaces(addRacesObj); @@ -122,6 +132,8 @@ public async Task AddsRacesToElection() public async Task HandlesAddRacesUnfoundElection() { var addRacesObj = new AddRacesModel { ElectionId = "blah", RaceIds = new List() { } }; + var validationResults = ValidationHelper.Validate(addRacesObj); + Assert.Empty(validationResults); var ret = await _electionApi.AddRaces(addRacesObj); Assert.NotNull(ret); @@ -138,6 +150,8 @@ public async Task HandlesAddRacesUnfoundElection() public async Task HandlesAddRacesUnfoundRace() { var addRacesObj = new AddRacesModel { ElectionId = "electionid1", RaceIds = new List { "68", "69", "70" } }; + var validationResults = ValidationHelper.Validate(addRacesObj); + Assert.Empty(validationResults); var ret = await _electionApi.AddRaces(addRacesObj); Assert.NotNull(ret); @@ -154,6 +168,8 @@ public async Task HandlesAddRacesUnfoundRace() public async Task HandlesAddRaceAlreadyInElection() { var addRacesObj = new AddRacesModel { ElectionId = "electionid1", RaceIds = new List { "raceid1", "raceid2", "raceid3" } }; + var validationResults = ValidationHelper.Validate(addRacesObj); + Assert.Empty(validationResults); var ret = await _electionApi.AddRaces(addRacesObj); Assert.NotNull(ret); diff --git a/TrueVote.Api.Tests/ServiceTests/Error500Test.cs b/TrueVote.Api.Tests/ServiceTests/Error500Test.cs index df5599c..55b579d 100644 --- a/TrueVote.Api.Tests/ServiceTests/Error500Test.cs +++ b/TrueVote.Api.Tests/ServiceTests/Error500Test.cs @@ -23,6 +23,8 @@ public Error500Test(ITestOutputHelper output) : base(output) public async Task LogsMessages() { var error500Flag = new Error500Flag { Error = false }; + var validationResults = ValidationHelper.Validate(error500Flag); + Assert.Empty(validationResults); await error500.ThrowError500(error500Flag); @@ -34,6 +36,8 @@ public async Task LogsMessages() public async Task CausesDivideByZero() { var error500Flag = new Error500Flag { Error = true }; + var validationResults = ValidationHelper.Validate(error500Flag); + Assert.Empty(validationResults); try { diff --git a/TrueVote.Api.Tests/ServiceTests/RaceTest.cs b/TrueVote.Api.Tests/ServiceTests/RaceTest.cs index e901fbf..d80ae1b 100644 --- a/TrueVote.Api.Tests/ServiceTests/RaceTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/RaceTest.cs @@ -37,6 +37,8 @@ public RaceTest(ITestOutputHelper output) : base(output) public async Task LogsMessages() { var baseRaceObj = new BaseRaceModel { Name = "President", RaceType = RaceTypes.ChooseOne }; + var validationResults = ValidationHelper.Validate(baseRaceObj); + Assert.Empty(validationResults); _ = await _raceApi.CreateRace(baseRaceObj); @@ -48,6 +50,8 @@ public async Task LogsMessages() public async Task AddsRace() { var baseRaceObj = new BaseRaceModel { Name = "President", RaceType = RaceTypes.ChooseOne }; + var validationResults = ValidationHelper.Validate(baseRaceObj); + Assert.Empty(validationResults); var ret = await _raceApi.CreateRace(baseRaceObj); Assert.NotNull(ret); @@ -77,6 +81,8 @@ public async Task AddsRace() public async Task FindsRace() { var findRaceObj = new FindRaceModel { Name = "President" }; + var validationResults = ValidationHelper.Validate(findRaceObj); + Assert.Empty(validationResults); var ret = await _raceApi.RaceFind(findRaceObj); Assert.NotNull(ret); @@ -97,6 +103,8 @@ public async Task FindsRace() public async Task HandlesUnfoundRace() { var findRaceObj = new FindRaceModel { Name = "not going to find anything" }; + var validationResults = ValidationHelper.Validate(findRaceObj); + Assert.Empty(validationResults); var ret = await _raceApi.RaceFind(findRaceObj); Assert.NotNull(ret); @@ -110,6 +118,8 @@ public async Task HandlesUnfoundRace() public async Task AddsCandidatesToRace() { var addCandidatesObj = new AddCandidatesModel { RaceId = "raceid3", CandidateIds = new List { MoqData.MockCandidateData[0].CandidateId, MoqData.MockCandidateData[1].CandidateId } }; + var validationResults = ValidationHelper.Validate(addCandidatesObj); + Assert.Empty(validationResults); var ret = await _raceApi.AddCandidates(addCandidatesObj); @@ -132,6 +142,8 @@ public async Task AddsCandidatesToRace() public async Task HandlesAddCandidatesUnfoundRace() { var addCandidatesObj = new AddCandidatesModel { RaceId = "blah", CandidateIds = new List() { } }; + var validationResults = ValidationHelper.Validate(addCandidatesObj); + Assert.Empty(validationResults); var ret = await _raceApi.AddCandidates(addCandidatesObj); Assert.NotNull(ret); @@ -148,6 +160,8 @@ public async Task HandlesAddCandidatesUnfoundRace() public async Task HandlesAddCandidatesUnfoundCandidate() { var addCandidatesObj = new AddCandidatesModel { RaceId = "raceid1", CandidateIds = new List { "68", "69" } }; + var validationResults = ValidationHelper.Validate(addCandidatesObj); + Assert.Empty(validationResults); var ret = await _raceApi.AddCandidates(addCandidatesObj); Assert.NotNull(ret); @@ -164,6 +178,8 @@ public async Task HandlesAddCandidatesUnfoundCandidate() public async Task HandlesAddCandidateAlreadyInRace() { var addCandidatesObj = new AddCandidatesModel { RaceId = "raceid1", CandidateIds = new List { "candidateid1", "candidateid2" } }; + var validationResults = ValidationHelper.Validate(addCandidatesObj); + Assert.Empty(validationResults); var ret = await _raceApi.AddCandidates(addCandidatesObj); Assert.NotNull(ret); diff --git a/TrueVote.Api.Tests/ServiceTests/UserTest.cs b/TrueVote.Api.Tests/ServiceTests/UserTest.cs index 75b0d84..7a91480 100644 --- a/TrueVote.Api.Tests/ServiceTests/UserTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/UserTest.cs @@ -35,6 +35,8 @@ public UserTest(ITestOutputHelper output) : base(output) public async Task LogsMessages() { var baseUserObj = new BaseUserModel { FullName = "Joe Blow", Email = "joe@joe.com", NostrPubKey = "nostr-key" }; + var validationResults = ValidationHelper.Validate(baseUserObj); + Assert.Empty(validationResults); _ = await _userApi.CreateUser(baseUserObj); @@ -46,6 +48,8 @@ public async Task LogsMessages() public async Task AddsUser() { var baseUserObj = new BaseUserModel { FullName = "Joe Blow", Email = "joe@joe.com", NostrPubKey = "nostr-key" }; + var validationResults = ValidationHelper.Validate(baseUserObj); + Assert.Empty(validationResults); var ret = await _userApi.CreateUser(baseUserObj); Assert.NotNull(ret); @@ -73,7 +77,9 @@ public async Task AddsUser() [Fact] public async Task FindsUser() { - var findUserObj = new FindUserModel { FullName = "Foo" }; + var findUserObj = new FindUserModel { FullName = "Foo Bar", Email = "foo@foo.com" }; + var validationResults = ValidationHelper.Validate(findUserObj); + Assert.Empty(validationResults); var ret = await _userApi.UserFind(findUserObj); Assert.NotNull(ret); @@ -81,9 +87,9 @@ public async Task FindsUser() var val = (UserModelList) (ret as OkObjectResult).Value; Assert.NotEmpty(val.Users); - Assert.Equal(2, val.Users.Count); - Assert.Equal("Foo2 Bar", val.Users[0].FullName); - Assert.Equal("foo2@bar.com", val.Users[0].Email); + Assert.Single(val.Users); + Assert.Equal("Foo Bar", val.Users[0].FullName); + Assert.Equal("foo@foo.com", val.Users[0].Email); _logHelper.Verify(LogLevel.Information, Times.Exactly(1)); _logHelper.Verify(LogLevel.Debug, Times.Exactly(2)); @@ -92,7 +98,9 @@ public async Task FindsUser() [Fact] public async Task HandlesUnfoundUser() { - var findUserObj = new FindUserModel { FullName = "not going to find anything" }; + var findUserObj = new FindUserModel { FullName = "not going to find anything", Email = "foo@whatever.com" }; + var validationResults = ValidationHelper.Validate(findUserObj); + Assert.Empty(validationResults); var ret = await _userApi.UserFind(findUserObj); Assert.NotNull(ret); @@ -114,8 +122,10 @@ public async Task HandlesSignInFailOnValidKeyInvalidSignature() PubKey = keyPair.PublicKey.Bech32, CreatedAt = utcTime.DateTime, Signature = "INVALID SIG", - Content = "" + Content = "CONTENT" }; + var validationResults = ValidationHelper.Validate(signInEventModel); + Assert.Empty(validationResults); var ret = await _userApi.SignIn(signInEventModel); @@ -140,8 +150,10 @@ public async Task HandlesSignInFailOnInvalidKey() PubKey = "INVALID KEY", CreatedAt = utcTime.DateTime, Signature = "INVALID SIG", - Content = "" + Content = "CONTENT" }; + var validationResults = ValidationHelper.Validate(signInEventModel); + Assert.Empty(validationResults); var ret = await _userApi.SignIn(signInEventModel); @@ -167,7 +179,10 @@ public async Task SignInSuccessUnfoundUser() Email = "unknown@truevote.org", FullName = "Joe Blow", NostrPubKey = keyPair.PublicKey.Bech32, - }; + }; + var validationResults = ValidationHelper.Validate(content); + Assert.Empty(validationResults); + var nostrEvent = new NostrEvent { Kind = NostrKind.ShortTextNote, @@ -188,6 +203,8 @@ public async Task SignInSuccessUnfoundUser() Content = nostrEvent.Content, Signature = signature.Sig }; + var validationResults2 = ValidationHelper.Validate(signInEventModel); + Assert.Empty(validationResults2); var ret = await _userApi.SignIn(signInEventModel); Assert.NotNull(ret); @@ -219,6 +236,8 @@ public async Task SignInSuccessFoundUser() NostrPubKey = keyPair.PublicKey.Bech32, UserPreferences = new UserPreferencesModel() }; + var validationResults = ValidationHelper.Validate(newUser); + Assert.Empty(validationResults); // Create own MoqData so it finds it later below var mockUserData = new List { newUser }; @@ -256,6 +275,8 @@ public async Task SignInSuccessFoundUser() Content = nostrEvent.Content, Signature = signature.Sig }; + var validationResults2 = ValidationHelper.Validate(signInEventModel); + Assert.Empty(validationResults2); var userApi = new User(_logHelper.Object, mockUserContext.Object, _mockServiceBus.Object, _mockJwtHandler.Object); var ret = await userApi.SignIn(signInEventModel); @@ -298,6 +319,8 @@ public async Task HandlesSignInSuccessFoundUserInvalidContent() Content = nostrEvent.Content, Signature = signature.Sig }; + var validationResults = ValidationHelper.Validate(signInEventModel); + Assert.Empty(validationResults); var ret = await _userApi.SignIn(signInEventModel); Assert.NotNull(ret); @@ -342,6 +365,8 @@ public async Task HandlesSignInFailOnValidKeyWrongSignature() Content = nostrEvent.Content, Signature = signature2.Sig }; + var validationResults = ValidationHelper.Validate(signInEventModel); + Assert.Empty(validationResults); var ret = await _userApi.SignIn(signInEventModel); Assert.NotNull(ret); @@ -361,6 +386,8 @@ public async Task SavesUser() user.FullName = "Joe Jones"; Assert.Equal(DateTime.MinValue, user.DateUpdated); Assert.Equal("Joe Jones", user.FullName); + var validationResults = ValidationHelper.Validate(user); + Assert.Empty(validationResults); _userApi.ControllerContext = _authControllerContext; var ret = await _userApi.SaveUser(user); @@ -385,6 +412,8 @@ public async Task SavesUserWithNewEmail() user.Email = "anewemail@anywhere.com"; Assert.Equal(DateTime.MinValue, user.DateUpdated); Assert.Equal("Joe Jones", user.FullName); + var validationResults = ValidationHelper.Validate(user); + Assert.Empty(validationResults); _userApi.ControllerContext = _authControllerContext; var ret = await _userApi.SaveUser(user); @@ -409,6 +438,8 @@ public async Task HandlesSavesUserWithoutAuthorization() user.FullName = "Joe Jones"; Assert.Equal(DateTime.MinValue, user.DateUpdated); Assert.Equal("Joe Jones", user.FullName); + var validationResults = ValidationHelper.Validate(user); + Assert.Empty(validationResults); var ret = await _userApi.SaveUser(user); @@ -426,6 +457,8 @@ public async Task HandlesSavesUserUserNotFound() user.FullName = "Joe Jones"; Assert.Equal(DateTime.MinValue, user.DateUpdated); Assert.Equal("Joe Jones", user.FullName); + var validationResults = ValidationHelper.Validate(user); + Assert.Empty(validationResults); _userApi.ControllerContext = _authControllerContext; var ret = await _userApi.SaveUser(user); @@ -440,6 +473,8 @@ public async Task HandlesSavesUserUserNotFound() public async Task SavesFeedback() { var feedback = MoqData.MockFeedbackData[0]; + var validationResults = ValidationHelper.Validate(feedback); + Assert.Empty(validationResults); _userApi.ControllerContext = _authControllerContext; var ret = await _userApi.SaveFeedback(feedback); @@ -460,6 +495,8 @@ public async Task HandlesSavesFeedbackUserNotFound() { var feedback = MoqData.MockFeedbackData[0]; feedback.UserId = "blah"; + var validationResults = ValidationHelper.Validate(feedback); + Assert.Empty(validationResults); _userApi.ControllerContext = _authControllerContext; var ret = await _userApi.SaveFeedback(feedback); @@ -474,6 +511,8 @@ public async Task HandlesSavesFeedbackUserNotFound() public async Task HandlesSavesFeedbackWithoutAuthorization() { var feedback = MoqData.MockFeedbackData[0]; + var validationResults = ValidationHelper.Validate(feedback); + Assert.Empty(validationResults); var ret = await _userApi.SaveFeedback(feedback); @@ -481,6 +520,20 @@ public async Task HandlesSavesFeedbackWithoutAuthorization() Assert.Equal(StatusCodes.Status401Unauthorized, ((IStatusCodeActionResult) ret).StatusCode); _logHelper.Verify(LogLevel.Debug, Times.Exactly(2)); + } + + [Fact] + public void EmailShouldReturnDefaultValueWhenNotSet() + { + var userModel = new BaseUserModel + { + FullName = "John Doe", + NostrPubKey = "some-public-key", + Email = "" + }; + + var email = userModel.Email; + Assert.Equal("unknown@truevote.org", email); } } } diff --git a/TrueVote.Api/Models/BallotModel.cs b/TrueVote.Api/Models/BallotModel.cs index 40b8270..96aa65f 100644 --- a/TrueVote.Api/Models/BallotModel.cs +++ b/TrueVote.Api/Models/BallotModel.cs @@ -2,7 +2,6 @@ using System.ComponentModel; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; -using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; using ByteConverter = TrueVote.Api.Helpers.ByteConverter; using JsonConverter = System.Text.Json.Serialization.JsonConverter; @@ -11,7 +10,6 @@ namespace TrueVote.Api.Models { - [ExcludeFromCodeCoverage] public class BallotList { [Required] @@ -29,7 +27,6 @@ public class BallotList public required List BallotHashes { get; set; } } - [ExcludeFromCodeCoverage] public class FindBallotModel { [Required] @@ -42,7 +39,6 @@ public class FindBallotModel public required string BallotId { get; set; } } - [ExcludeFromCodeCoverage] public class CountBallotModel { [Required] @@ -60,7 +56,6 @@ public class CountBallotModel public required DateTime DateCreatedEnd { get; set; } } - [ExcludeFromCodeCoverage] public class CountBallotModelResponse { [Required] @@ -71,7 +66,6 @@ public class CountBallotModelResponse public required long BallotCount { get; set; } } - [ExcludeFromCodeCoverage] public class BallotModel { [Required] @@ -98,7 +92,6 @@ public class BallotModel public required DateTime DateCreated { get; set; } } - [ExcludeFromCodeCoverage] public class SubmitBallotModel { [Required] [Description("Election")] @@ -113,7 +106,6 @@ public class SubmitBallotModel { // public string UserIdBallotIdHashed { get; set; } } - [ExcludeFromCodeCoverage] public class SubmitBallotModelResponse { [Required] [Description("Ballot Id")] @@ -141,7 +133,6 @@ public class SubmitBallotModelResponse { public required string Message { get; set; } } - [ExcludeFromCodeCoverage] public class BallotHashModel { [Required] @@ -202,7 +193,6 @@ public class BallotHashModel public string? TimestampId { get; set; } } - [ExcludeFromCodeCoverage] public class FindBallotHashModel { [Required] diff --git a/TrueVote.Api/Models/CandidateModel.cs b/TrueVote.Api/Models/CandidateModel.cs index d31f0e1..b2a6f46 100644 --- a/TrueVote.Api/Models/CandidateModel.cs +++ b/TrueVote.Api/Models/CandidateModel.cs @@ -1,14 +1,12 @@ using Newtonsoft.Json; using System.ComponentModel; using System.ComponentModel.DataAnnotations; -using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; using TrueVote.Api.Helpers; using JsonIgnore = Newtonsoft.Json.JsonIgnoreAttribute; namespace TrueVote.Api.Models { - [ExcludeFromCodeCoverage] public class CandidateObj { [JsonPropertyName("Candidate")] @@ -16,7 +14,6 @@ public class CandidateObj public List? candidate; } - [ExcludeFromCodeCoverage] public class CandidateModelList { [Required] @@ -27,7 +24,6 @@ public class CandidateModelList public required List Candidates { get; set; } } - [ExcludeFromCodeCoverage] public class FindCandidateModel { [Required] @@ -46,7 +42,6 @@ public class FindCandidateModel public string PartyAffiliation { get; set; } = string.Empty; } - [ExcludeFromCodeCoverage] public class BaseCandidateModel { [Required] @@ -66,7 +61,6 @@ public class BaseCandidateModel public required string PartyAffiliation { get; set; } = string.Empty; } - [ExcludeFromCodeCoverage] public class CandidateModel { [Required] diff --git a/TrueVote.Api/Models/ElectionModel.cs b/TrueVote.Api/Models/ElectionModel.cs index 7c29846..70ac8ef 100644 --- a/TrueVote.Api/Models/ElectionModel.cs +++ b/TrueVote.Api/Models/ElectionModel.cs @@ -1,22 +1,10 @@ using Newtonsoft.Json; using System.ComponentModel; using System.ComponentModel.DataAnnotations; -using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; -using TrueVote.Api.Helpers; -using JsonIgnore = Newtonsoft.Json.JsonIgnoreAttribute; namespace TrueVote.Api.Models { - [ExcludeFromCodeCoverage] - public class ElectionObj - { - [JsonPropertyName("Election")] - [JsonProperty("Election", Required = Required.Default)] - public List? election; - } - - [ExcludeFromCodeCoverage] public class ElectionModelList { [Required] @@ -27,7 +15,6 @@ public class ElectionModelList public required List Elections { get; set; } } - [ExcludeFromCodeCoverage] public class FindElectionModel { [Required] @@ -39,7 +26,6 @@ public class FindElectionModel public required string Name { get; set; } = string.Empty; } - [ExcludeFromCodeCoverage] public class BaseElectionModel { [Required] @@ -88,7 +74,6 @@ public class BaseElectionModel public required List Races { get; set; } = new List(); } - [ExcludeFromCodeCoverage] public class ElectionModel { [Required] @@ -160,74 +145,6 @@ public class ElectionModel public required List Races { get; set; } = new List(); } - // Same as above model but without required properties - [ExcludeFromCodeCoverage] - public class ElectionModelResponse - { - [Required] - [Description("Election Id")] - [MaxLength(2048)] - [DataType(DataType.Text)] - [JsonPropertyName("ElectionId")] - [JsonProperty(nameof(ElectionId), Required = Required.Always)] - [Key] - [JsonIgnore] - public required string ElectionId { get; set; } = Guid.NewGuid().ToString(); - - [Description("Parent Election Id")] - [MaxLength(2048)] - [DataType(DataType.Text)] - [JsonPropertyName("ParentElectionId")] - [JsonProperty(nameof(ParentElectionId), Required = Required.Default)] - public string? ParentElectionId { get; set; } - - [Required] - [Description("Name")] - [MaxLength(2048)] - [DataType(DataType.Text)] - [JsonPropertyName("Name")] - [JsonProperty(nameof(Name), Required = Required.Always)] - public required string Name { get; set; } = string.Empty; - - [Required] - [Description("Description")] - [MaxLength(32768)] - [DataType(DataType.Text)] - [JsonPropertyName("Description")] - [JsonProperty(nameof(Description), Required = Required.Always)] - public required string Description { get; set; } = string.Empty; - - [Required] - [Description("StartDate")] - [DataType(DataType.Date)] - [JsonPropertyName("StartDate")] - [JsonProperty(nameof(StartDate), Required = Required.Always)] - public required DateTime StartDate { get; set; } - - [Required] - [Description("EndDate")] - [DataType(DataType.Date)] - [JsonPropertyName("EndDate")] - [JsonProperty(nameof(EndDate), Required = Required.Always)] - public required DateTime EndDate { get; set; } - - [Required] - [Description("DateCreated")] - [DataType(DataType.Date)] - [JsonPropertyName("DateCreated")] - [JsonProperty(nameof(DateCreated), Required = Required.Always)] - [JsonIgnore] - public required DateTime DateCreated { get; set; } = UtcNowProviderFactory.GetProvider().UtcNow; - - [Required] - [Description("List of Races")] - [DataType("List")] - [JsonPropertyName("Races")] - [JsonProperty(nameof(Races), Required = Required.Always)] - public required List Races { get; set; } = new List(); - } - - [ExcludeFromCodeCoverage] public class AddRacesModel { [Required] @@ -243,6 +160,6 @@ public class AddRacesModel [Description("Race Ids")] [JsonPropertyName("RaceIds")] [JsonProperty(nameof(RaceIds), Required = Required.Always)] - public required List RaceIds { get; set; } + public required List RaceIds { get; set; } = new List(); } } diff --git a/TrueVote.Api/Models/FeedbackModel.cs b/TrueVote.Api/Models/FeedbackModel.cs index c53508f..6bdc6aa 100644 --- a/TrueVote.Api/Models/FeedbackModel.cs +++ b/TrueVote.Api/Models/FeedbackModel.cs @@ -1,14 +1,12 @@ using Newtonsoft.Json; using System.ComponentModel.DataAnnotations; using System.ComponentModel; -using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; using JsonIgnore = Newtonsoft.Json.JsonIgnoreAttribute; using TrueVote.Api.Helpers; namespace TrueVote.Api.Models { - [ExcludeFromCodeCoverage] public class FeedbackModel { [Required] diff --git a/TrueVote.Api/Models/HelperModel.cs b/TrueVote.Api/Models/HelperModel.cs index 64e1de4..025c580 100644 --- a/TrueVote.Api/Models/HelperModel.cs +++ b/TrueVote.Api/Models/HelperModel.cs @@ -1,12 +1,10 @@ using Newtonsoft.Json; using System.ComponentModel; using System.ComponentModel.DataAnnotations; -using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; namespace TrueVote.Api.Models { - [ExcludeFromCodeCoverage] public class SecureString { [Required] @@ -18,7 +16,6 @@ public class SecureString public required string Value { get; set; } = string.Empty; } - [ExcludeFromCodeCoverage] public class Error500Flag { [Required] diff --git a/TrueVote.Api/Models/RaceModel.cs b/TrueVote.Api/Models/RaceModel.cs index 62c0006..cfd558e 100644 --- a/TrueVote.Api/Models/RaceModel.cs +++ b/TrueVote.Api/Models/RaceModel.cs @@ -2,7 +2,6 @@ using System.ComponentModel; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; -using System.Diagnostics.CodeAnalysis; using System.Runtime.Serialization; using System.Text.Json.Serialization; using TrueVote.Api.Helpers; @@ -22,14 +21,6 @@ public enum RaceTypes RankedChoice = 2 } - [ExcludeFromCodeCoverage] - public class RaceObj - { - [JsonPropertyName("Race")] - public List? race; - } - - [ExcludeFromCodeCoverage] public class RaceModelList { [Required] @@ -39,7 +30,6 @@ public class RaceModelList public required List Races { get; set; } } - [ExcludeFromCodeCoverage] public class FindRaceModel { [Required] @@ -51,7 +41,6 @@ public class FindRaceModel public required string Name { get; set; } = string.Empty; } - [ExcludeFromCodeCoverage] public class BaseRaceModel { [Required] @@ -70,7 +59,6 @@ public class BaseRaceModel public required RaceTypes RaceType { get; set; } } - [ExcludeFromCodeCoverage] public class RaceModel { [Required] @@ -130,61 +118,6 @@ public class RaceModel public List Candidates { get; set; } = new List(); } - // Same as above model but without required properties - [ExcludeFromCodeCoverage] - public class RaceModelResponse - { - [Required] - [Description("Race Id")] - [MaxLength(2048)] - [DataType(DataType.Text)] - [JsonPropertyName("RaceId")] - [JsonProperty(nameof(RaceId), Required = Required.Always)] - [Key] - [JsonIgnore] - public required string RaceId { get; set; } = Guid.NewGuid().ToString(); - - [Required] - [Description("Name")] - [MaxLength(2048)] - [DataType(DataType.Text)] - [JsonPropertyName("Name")] - [JsonProperty(nameof(Name), Required = Required.Always)] - public required string Name { get; set; } = string.Empty; - - [Required] - [Description("Race Type")] - [EnumDataType(typeof(RaceTypes))] - [JsonPropertyName("RaceType")] - [JsonProperty(nameof(RaceType), Required = Required.Always)] - public required RaceTypes RaceType { get; set; } - - [Required] - [Description("Race Type Name")] - [MaxLength(2048)] - [DataType(DataType.Text)] - [NotMapped] - [JsonPropertyName("RaceTypeName")] - [JsonProperty(nameof(RaceTypeName), Required = Required.Always)] - public string RaceTypeName => RaceType.ToString(); - - [Required] - [Description("DateCreated")] - [DataType(DataType.Date)] - [JsonPropertyName("DateCreated")] - [JsonProperty(nameof(DateCreated), Required = Required.Always)] - [JsonIgnore] - public required DateTime DateCreated { get; set; } = UtcNowProviderFactory.GetProvider().UtcNow; - - [Required] - [Description("List of Candidates")] - [DataType("List")] - [JsonPropertyName("Candidates")] - [JsonProperty(nameof(Candidates), Required = Required.Always)] - public required List Candidates { get; set; } = new List(); - } - - [ExcludeFromCodeCoverage] public class AddCandidatesModel { [Required] diff --git a/TrueVote.Api/Models/StatusModel.cs b/TrueVote.Api/Models/StatusModel.cs index b51c2f7..fd1fd35 100644 --- a/TrueVote.Api/Models/StatusModel.cs +++ b/TrueVote.Api/Models/StatusModel.cs @@ -1,12 +1,10 @@ using Newtonsoft.Json; using System.ComponentModel; using System.ComponentModel.DataAnnotations; -using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; namespace TrueVote.Api.Models { - [ExcludeFromCodeCoverage] public class BuildInfo { [Description("Git branch of instance")] @@ -34,7 +32,6 @@ public class BuildInfo public string Commit { get; set; } = string.Empty; } - [ExcludeFromCodeCoverage] public class StatusModel { [Description("Current Time")] diff --git a/TrueVote.Api/Models/TimestampModel.cs b/TrueVote.Api/Models/TimestampModel.cs index d2dc095..ca4160a 100644 --- a/TrueVote.Api/Models/TimestampModel.cs +++ b/TrueVote.Api/Models/TimestampModel.cs @@ -1,12 +1,10 @@ using Newtonsoft.Json; using System.ComponentModel; using System.ComponentModel.DataAnnotations; -using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; namespace TrueVote.Api.Models { - [ExcludeFromCodeCoverage] public class TimestampModel { [Required] @@ -65,7 +63,6 @@ public class TimestampModel public required DateTime DateCreated { get; set; } } - [ExcludeFromCodeCoverage] public class FindTimestampModel { [Required] diff --git a/TrueVote.Api/Models/UserModel.cs b/TrueVote.Api/Models/UserModel.cs index 950a099..f17e0bd 100644 --- a/TrueVote.Api/Models/UserModel.cs +++ b/TrueVote.Api/Models/UserModel.cs @@ -1,6 +1,5 @@ using Newtonsoft.Json; using System.ComponentModel.DataAnnotations; -using System.Diagnostics.CodeAnalysis; using Nostr.Client.Messages; using System.ComponentModel; using System.Text.Json.Serialization; @@ -8,7 +7,6 @@ namespace TrueVote.Api.Models { - [ExcludeFromCodeCoverage] public class UserObj { [JsonPropertyName("User")] @@ -16,7 +14,6 @@ public class UserObj public List? user; } - [ExcludeFromCodeCoverage] public class UserModelList { [Required] @@ -27,7 +24,6 @@ public class UserModelList public required List Users { get; set; } } - [ExcludeFromCodeCoverage] public class FindUserModel { [Required] @@ -48,7 +44,6 @@ public class FindUserModel public string Email { get; set; } = string.Empty; } - [ExcludeFromCodeCoverage] public class BaseUserModel { [Required] @@ -84,7 +79,6 @@ public required string Email public required string NostrPubKey { get; set; } } - [ExcludeFromCodeCoverage] public class UserModel { [Required] @@ -145,7 +139,6 @@ public class UserModel public required UserPreferencesModel UserPreferences { get; set; } = new UserPreferencesModel(); } - [ExcludeFromCodeCoverage] public class SignInResponse { [Required] @@ -164,7 +157,6 @@ public class SignInResponse public required string Token { get; set; } } - [ExcludeFromCodeCoverage] public class UserPreferencesModel { [Description("Notification: New Elections")] @@ -188,7 +180,6 @@ public class UserPreferencesModel public bool NotificationNewTrueVoteFeatures { get; set; } } - [ExcludeFromCodeCoverage] public class SignInEventModel { [Required] From b18bc19684a073865c1cdd37712b328d0c2b0ef4 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Thu, 30 May 2024 07:39:44 -0700 Subject: [PATCH 08/19] AD-99 DRYd out Custom Choice Validator and made it unique for Candidates.Where(selected == true) --- .../HelperTests/CustomValidatorTest.cs | 202 ++++++++++++------ .../HelperTests/MerkleTreeTest.cs | 4 +- TrueVote.Api.Tests/ServiceTests/BallotTest.cs | 48 ++++- TrueVote.Api/Helpers/CustomValidators.cs | 61 ++++-- TrueVote.Api/Models/RaceModel.cs | 18 +- 5 files changed, 233 insertions(+), 100 deletions(-) diff --git a/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs b/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs index 6c83e78..a9c86e7 100644 --- a/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs +++ b/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs @@ -5,137 +5,197 @@ using TrueVote.Api.Tests.Helpers; using TrueVote.Api.Helpers; using Newtonsoft.Json; +using System.Text.Json.Serialization; +using TrueVote.Api.Models; using System.Linq; namespace TrueVote.Api.Tests.HelperTests { - public class NameModel + public class CandidateTestModel { - [Required] - [Description("Name")] - [MaxLength(2048)] - [DataType(DataType.Text)] - public required string Name { get; set; } = string.Empty; - } + [Description("List of Candidates")] + [DataType("List")] + [JsonPropertyName("Candidates")] + [JsonProperty(nameof(Candidates), Required = Required.Default)] + public List Candidates { get; set; } = new List(); - public class TestModel1 - { - [Description("Number of Choices")] + [Description("Max Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [NumberOfChoicesValidator(nameof(NameModel))] - public int? NumberOfChoices { get; set; } + [MaxNumberOfChoicesValidator(nameof(Candidates))] + public int? MaxNumberOfChoices { get; set; } - [Description("List of Names")] - [DataType("List")] - public List NameModel { get; set; } = new List(); + [Description("Min Number of Choices")] + [DataType("integer")] + [Range(0, int.MaxValue)] + [MinNumberOfChoicesValidator(nameof(Candidates))] + public int? MinNumberOfChoices { get; set; } } - public class TestModel2 + public class CandidateTestModelBlankProperty { - [Description("Number of Choices")] + [Description("List of Candidates")] + [DataType("List")] + [JsonPropertyName("Candidates")] + [JsonProperty(nameof(Candidates), Required = Required.Default)] + public List Candidates { get; set; } = new List(); + + [Description("Max Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [NumberOfChoicesValidator("")] - public int? NumberOfChoices { get; set; } + [MaxNumberOfChoicesValidator("")] + public int? MaxNumberOfChoices { get; set; } - [Description("List of Names")] - [DataType("List")] - public List NameModel { get; set; } = new List(); + [Description("Min Number of Choices")] + [DataType("integer")] + [Range(0, int.MaxValue)] + [MinNumberOfChoicesValidator("")] + public int? MinNumberOfChoices { get; set; } } - public class TestModel3 + public class CandidateTestModelMinInvalidProperty { - [Description("Number of Choices")] + [Description("List of Candidates")] + [DataType("List")] + [JsonPropertyName("Candidates")] + [JsonProperty(nameof(Candidates), Required = Required.Default)] + public string Candidates { get; set; } + + [Description("Min Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [NumberOfChoicesValidator(nameof(Name))] - public int? NumberOfChoices { get; set; } + [MinNumberOfChoicesValidator(nameof(Candidates))] + public int? MinNumberOfChoices { get; set; } + } + + public class CandidateTestModelMaxInvalidProperty + { + [Description("List of Candidates")] + [DataType("List")] + [JsonPropertyName("Candidates")] + [JsonProperty(nameof(Candidates), Required = Required.Default)] + public string Candidates { get; set; } - [Description("Name")] - [DataType(DataType.Text)] - public string Name { get; set; } = string.Empty; + [Description("Max Number of Choices")] + [DataType("integer")] + [Range(0, int.MaxValue)] + [MaxNumberOfChoicesValidator(nameof(Candidates))] + public int? MaxNumberOfChoices { get; set; } } - public class TestModel5 + public class CandidateTestModelMaxNotFoundProperty { - [Description("Number of Choices")] + [Description("Max Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [NumberOfChoicesValidator(nameof(NameModel))] - public int? NumberOfChoices { get; set; } + [MaxNumberOfChoicesValidator("foo")] + public int? MaxNumberOfChoices { get; set; } + } - [Description("List of Names")] - [DataType("List")] - public List NameModel { get; set; } = new List(); + public class CandidateTestModelMinNotFoundProperty + { + [Description("Min Number of Choices")] + [DataType("integer")] + [Range(0, int.MaxValue)] + [MaxNumberOfChoicesValidator("foo")] + public int? MinNumberOfChoices { get; set; } } public class CustomValidatorTest { [Fact] - public void TestsNumberOfChoicesSucceeds() + public void TestsMinAndMaxNumberOfChoicesSucceeds() { - var testModel = new TestModel1 { NumberOfChoices = 1 }; - testModel.NameModel.Add(new NameModel { Name = "name1 "}); - testModel.NameModel.Add(new NameModel { Name = "name2 " }); - testModel.NameModel.Add(new NameModel { Name = "name3 " }); + var testModel = new CandidateTestModel { MaxNumberOfChoices = 1, MinNumberOfChoices = 1, Candidates = MoqData.MockCandidateData }; + testModel.Candidates[0].Selected = true; + Assert.Single(testModel.Candidates.Where(c => c.Selected == true)); var validationResults = ValidationHelper.Validate(testModel); Assert.Empty(validationResults); } [Fact] - public void TestsNumberOfChoicesFailsWhenTooHigh() + public void TestsMinNumberOfChoicesFails() { - var testModel = new TestModel1 { NumberOfChoices = 5 }; - testModel.NameModel.Add(new NameModel { Name = "name1 " }); - testModel.NameModel.Add(new NameModel { Name = "name2 " }); - testModel.NameModel.Add(new NameModel { Name = "name3 " }); + var testModel = new CandidateTestModel { MinNumberOfChoices = 3, Candidates = MoqData.MockCandidateData }; + testModel.Candidates[0].Selected = true; + testModel.Candidates[1].Selected = true; + Assert.Equal(2, testModel.Candidates.Where(c => c.Selected == true).Count()); var validationResults = ValidationHelper.Validate(testModel); - Assert.NotEmpty(validationResults); - Assert.NotNull(validationResults); - Assert.Single(validationResults); - Assert.Contains("NumberOfChoices cannot exceed the", validationResults[0].ErrorMessage); - Assert.Equal("NumberOfChoices", validationResults[0].MemberNames.First()); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("must be greater or equal to MinNumberOfChoices", validationResults[0].ErrorMessage); + Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); } [Fact] - public void TestsNumberOfChoicesFailsWhenPropertyNotFound() + public void TestsMaxNumberOfChoicesFails() { - var testModel = new TestModel2 { NumberOfChoices = 1 }; - testModel.NameModel.Add(new NameModel { Name = "name1 " }); - testModel.NameModel.Add(new NameModel { Name = "name2 " }); - testModel.NameModel.Add(new NameModel { Name = "name3 " }); + var testModel = new CandidateTestModel { MaxNumberOfChoices = 1, Candidates = MoqData.MockCandidateData }; + testModel.Candidates[0].Selected = true; + testModel.Candidates[1].Selected = true; + Assert.Equal(2, testModel.Candidates.Where(c => c.Selected == true).Count()); var validationResults = ValidationHelper.Validate(testModel); - Assert.NotEmpty(validationResults); - Assert.NotNull(validationResults); - Assert.Single(validationResults); - Assert.Contains("Property not found", validationResults[0].ErrorMessage); - Assert.Equal("NumberOfChoices", validationResults[0].MemberNames.First()); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("cannot exceed MaxNumberOfChoices", validationResults[0].ErrorMessage); + Assert.Equal("MaxNumberOfChoices", validationResults[0].MemberNames.First()); } [Fact] - public void TestsNumberOfChoicesFailsWhenPropertyToCheckIsInvalid() + public void TestsMinNumberOfChoicesInvalidProperty() { - var testModel = new TestModel3 { NumberOfChoices = 1 }; + var testModel = new CandidateTestModelMinInvalidProperty { MinNumberOfChoices = 3, Candidates = "foo" }; var validationResults = ValidationHelper.Validate(testModel); - Assert.NotEmpty(validationResults); - Assert.NotNull(validationResults); - Assert.Single(validationResults); - Assert.Contains("is not a valid collection", validationResults[0].ErrorMessage); - Assert.Equal("NumberOfChoices", validationResults[0].MemberNames.First()); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("Property 'Candidates' is not a valid List type", validationResults[0].ErrorMessage); + Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); } [Fact] - public void TestsNumberOfChoicesSucceedsWhenNumberOfChoicesIsUnset() + public void TestsMaxNumberOfChoicesInvalidProperty() { - var testModel = new TestModel5(); + var testModel = new CandidateTestModelMaxInvalidProperty { MaxNumberOfChoices = 3, Candidates = "foo" }; var validationResults = ValidationHelper.Validate(testModel); - Assert.Empty(validationResults); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("Property 'Candidates' is not a valid List type", validationResults[0].ErrorMessage); + Assert.Equal("MaxNumberOfChoices", validationResults[0].MemberNames.First()); + } + + [Fact] + public void TestsNumberOfChoicesMaxNotFoundProperty() + { + var testModel = new CandidateTestModelMaxNotFoundProperty { MaxNumberOfChoices = 3 }; + + var validationResults = ValidationHelper.Validate(testModel); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("Property not found", validationResults[0].ErrorMessage); + Assert.Equal("MaxNumberOfChoices", validationResults[0].MemberNames.First()); + } + + [Fact] + public void TestsNumberOfChoicesMinNotFoundProperty() + { + var testModel = new CandidateTestModelMinNotFoundProperty { MinNumberOfChoices = 3 }; + + var validationResults = ValidationHelper.Validate(testModel); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("Property not found", validationResults[0].ErrorMessage); + Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); } } } diff --git a/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs b/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs index b8a2c3b..b58d461 100644 --- a/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs +++ b/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs @@ -70,8 +70,8 @@ public void CreatesHashFromBallotObject() Assert.NotNull(hash1); Assert.NotNull(hash2); Assert.Equal(hash1, hash2); - Assert.Equal("4", hash1[0].ToString()); - Assert.Equal("4", hash2[0].ToString()); + Assert.Equal("67", hash1[0].ToString()); + Assert.Equal("67", hash2[0].ToString()); } [Fact] diff --git a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs index 36fac92..faf74b6 100644 --- a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs @@ -47,32 +47,64 @@ public async Task SubmitsBallot() _logHelper.Verify(LogLevel.Debug, Times.Exactly(2)); } + // TODO Test needs to look at selected choices, not just number of total choices [Fact] - public void SubmitsBallotWithInvalidNumberOfChoices() + public void SubmitsBallotWithInvalidNumberOfMaxChoices() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; - baseBallotObj.Election.Races[0].NumberOfChoices = -1; + baseBallotObj.Election.Races[0].MaxNumberOfChoices = -1; var validationResults = ValidationHelper.Validate(baseBallotObj); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); Assert.Single(validationResults); - Assert.Contains("NumberOfChoices must be between 0", validationResults[0].ErrorMessage); - Assert.Equal("NumberOfChoices", validationResults[0].MemberNames.First()); + Assert.Contains("MaxNumberOfChoices must be between 0", validationResults[0].ErrorMessage); + Assert.Equal("MaxNumberOfChoices", validationResults[0].MemberNames.First()); } + // TODO Test needs to look at selected choices, not just number of total choices [Fact] - public void SubmitsBallotWithAboveCandidateCountNumberOfChoices() + public void SubmitsBallotWithAboveCandidateCountNumberOfMaxChoices() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; - baseBallotObj.Election.Races[0].NumberOfChoices = 20; + baseBallotObj.Election.Races[0].MaxNumberOfChoices = 20; var validationResults = ValidationHelper.Validate(baseBallotObj); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); Assert.Single(validationResults); - Assert.Contains("NumberOfChoices cannot exceed the", validationResults[0].ErrorMessage); - Assert.Equal("NumberOfChoices", validationResults[0].MemberNames.First()); + Assert.Contains("MaxNumberOfChoices cannot exceed the", validationResults[0].ErrorMessage); + Assert.Equal("MaxNumberOfChoices", validationResults[0].MemberNames.First()); + } + + // TODO Test needs to look at selected choices, not just number of total choices + [Fact] + public void SubmitsBallotWithInvalidNumberOfMinChoices() + { + var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; + baseBallotObj.Election.Races[0].MinNumberOfChoices = -1; + + var validationResults = ValidationHelper.Validate(baseBallotObj); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("MinNumberOfChoices must be between 0", validationResults[0].ErrorMessage); + Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); + } + + // TODO Test needs to look at selected choices, not just number of total choices + [Fact] + public void SubmitsBallotWithBelowCandidateCountNumberOfMinChoices() + { + var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; + baseBallotObj.Election.Races[0].MinNumberOfChoices = 20; + + var validationResults = ValidationHelper.Validate(baseBallotObj); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("MinNumberOfChoices must be greater or equal to", validationResults[0].ErrorMessage); + Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); } [Fact] diff --git a/TrueVote.Api/Helpers/CustomValidators.cs b/TrueVote.Api/Helpers/CustomValidators.cs index 2c44117..59f9626 100644 --- a/TrueVote.Api/Helpers/CustomValidators.cs +++ b/TrueVote.Api/Helpers/CustomValidators.cs @@ -1,42 +1,75 @@ #pragma warning disable IDE0046 // Convert to conditional expression using System.Collections; using System.ComponentModel.DataAnnotations; +using TrueVote.Api.Models; namespace TrueVote.Api.Helpers { - public class NumberOfChoicesValidatorAttribute : ValidationAttribute + public abstract class NumberOfChoicesValidatorAttribute : ValidationAttribute { - private readonly string _propertyName; + protected readonly string CandidatesPropertyName; - public NumberOfChoicesValidatorAttribute(string propertyName) + protected NumberOfChoicesValidatorAttribute(string propertyName) { - _propertyName = propertyName; + CandidatesPropertyName = propertyName; } protected override ValidationResult IsValid(object value, ValidationContext validationContext) { // Get the property info of the property - var propertyInfo = validationContext.ObjectType.GetProperty(_propertyName); + var candidatePropertyInfo = validationContext.ObjectType.GetProperty(CandidatesPropertyName); - if (propertyInfo == null) + if (candidatePropertyInfo == null) { return new ValidationResult($"Property not found.", [validationContext.MemberName]); } // Get the value of the property - var propertyValue = propertyInfo.GetValue(validationContext.ObjectInstance); - if (propertyValue is not IEnumerable or string or int or long or DateTime) + var candidatePropertyValue = candidatePropertyInfo.GetValue(validationContext.ObjectInstance); + if (candidatePropertyValue is not List) { - return new ValidationResult($"Property '{_propertyName}' is not a valid collection.", [validationContext.MemberName]); + return new ValidationResult($"Property '{CandidatesPropertyName}' is not a valid List type.", [validationContext.MemberName]); } - // Calculate the count of the collection - var count = ((IEnumerable) propertyValue).Cast().Count(); + // Calculate the number of selections in the candidate choices + var count = ((IEnumerable) candidatePropertyValue).Cast().Where(c => c.Selected == true).Count(); - var numberOfChoices = value as int?; - if (numberOfChoices.HasValue && numberOfChoices.Value > count) + return ValidateCount(value, count, validationContext); + } + + protected abstract ValidationResult ValidateCount(object value, int count, ValidationContext validationContext); + } + + public class MaxNumberOfChoicesValidatorAttribute : NumberOfChoicesValidatorAttribute + { + public MaxNumberOfChoicesValidatorAttribute(string propertyName) : base(propertyName) + { + } + + protected override ValidationResult ValidateCount(object value, int count, ValidationContext validationContext) + { + var maxNumberOfChoices = value as int?; + if (maxNumberOfChoices.HasValue && (count > maxNumberOfChoices.Value)) + { + return new ValidationResult($"Number of selected items in '{CandidatesPropertyName}' cannot exceed MaxNumberOfChoices. MaxNumberOfChoices: {maxNumberOfChoices}, Count: {count}", [validationContext.MemberName]); + } + + return ValidationResult.Success; + } + } + + public class MinNumberOfChoicesValidatorAttribute : NumberOfChoicesValidatorAttribute + { + public MinNumberOfChoicesValidatorAttribute(string propertyName) : base(propertyName) + { + } + + protected override ValidationResult ValidateCount(object value, int count, ValidationContext validationContext) + { + var minNumberOfChoices = value as int?; + if (minNumberOfChoices.HasValue && (count < minNumberOfChoices.Value)) { - return new ValidationResult($"NumberOfChoices cannot exceed the number of items in '{_propertyName}'. NumberOfChoices: {numberOfChoices}, Count: {count}", [validationContext.MemberName]); + return new ValidationResult($"Number of selected items in '{CandidatesPropertyName}' must be greater or equal to MinNumberOfChoices. MinNumberOfChoices: {minNumberOfChoices}, Count: {count}", [validationContext.MemberName]); } return ValidationResult.Success; diff --git a/TrueVote.Api/Models/RaceModel.cs b/TrueVote.Api/Models/RaceModel.cs index cfd558e..96c4ad5 100644 --- a/TrueVote.Api/Models/RaceModel.cs +++ b/TrueVote.Api/Models/RaceModel.cs @@ -95,13 +95,21 @@ public class RaceModel [JsonProperty(nameof(RaceTypeName), Required = Required.Default)] public string RaceTypeName => RaceType.ToString(); - [Description("Number of Choices")] + [Description("Max Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [JsonPropertyName("NumberOfChoices")] - [JsonProperty(nameof(NumberOfChoices), Required = Required.Default)] - [NumberOfChoicesValidator(nameof(Candidates))] - public int? NumberOfChoices { get; set; } + [JsonPropertyName("MaxNumberOfChoices")] + [JsonProperty(nameof(MaxNumberOfChoices), Required = Required.Default)] + [MaxNumberOfChoicesValidator(nameof(Candidates))] + public int? MaxNumberOfChoices { get; set; } + + [Description("Min Number of Choices")] + [DataType("integer")] + [Range(0, int.MaxValue)] + [JsonPropertyName("MinNumberOfChoices")] + [JsonProperty(nameof(MinNumberOfChoices), Required = Required.Default)] + [MinNumberOfChoicesValidator(nameof(Candidates))] + public int? MinNumberOfChoices { get; set; } [Required] [Description("DateCreated")] From e59b98f73ba58802c40d82faaf274d7d08e3f9fc Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Thu, 30 May 2024 08:00:43 -0700 Subject: [PATCH 09/19] AD-99 Updated BallotSubmission tests to check for 'selected' --- TrueVote.Api.Tests/ServiceTests/BallotTest.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs index faf74b6..cce9702 100644 --- a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs @@ -47,7 +47,6 @@ public async Task SubmitsBallot() _logHelper.Verify(LogLevel.Debug, Times.Exactly(2)); } - // TODO Test needs to look at selected choices, not just number of total choices [Fact] public void SubmitsBallotWithInvalidNumberOfMaxChoices() { @@ -57,27 +56,27 @@ public void SubmitsBallotWithInvalidNumberOfMaxChoices() var validationResults = ValidationHelper.Validate(baseBallotObj); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); - Assert.Single(validationResults); + // Assert.Single(validationResults); Assert.Contains("MaxNumberOfChoices must be between 0", validationResults[0].ErrorMessage); Assert.Equal("MaxNumberOfChoices", validationResults[0].MemberNames.First()); } - // TODO Test needs to look at selected choices, not just number of total choices [Fact] public void SubmitsBallotWithAboveCandidateCountNumberOfMaxChoices() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; - baseBallotObj.Election.Races[0].MaxNumberOfChoices = 20; + baseBallotObj.Election.Races[0].MaxNumberOfChoices = 1; + baseBallotObj.Election.Races[0].Candidates[0].Selected = true; + baseBallotObj.Election.Races[0].Candidates[1].Selected = true; var validationResults = ValidationHelper.Validate(baseBallotObj); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); Assert.Single(validationResults); - Assert.Contains("MaxNumberOfChoices cannot exceed the", validationResults[0].ErrorMessage); + Assert.Contains("cannot exceed MaxNumberOfChoices", validationResults[0].ErrorMessage); Assert.Equal("MaxNumberOfChoices", validationResults[0].MemberNames.First()); } - // TODO Test needs to look at selected choices, not just number of total choices [Fact] public void SubmitsBallotWithInvalidNumberOfMinChoices() { @@ -92,18 +91,18 @@ public void SubmitsBallotWithInvalidNumberOfMinChoices() Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); } - // TODO Test needs to look at selected choices, not just number of total choices [Fact] public void SubmitsBallotWithBelowCandidateCountNumberOfMinChoices() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; - baseBallotObj.Election.Races[0].MinNumberOfChoices = 20; + baseBallotObj.Election.Races[0].MinNumberOfChoices = 2; + baseBallotObj.Election.Races[0].Candidates[0].Selected = true; var validationResults = ValidationHelper.Validate(baseBallotObj); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); Assert.Single(validationResults); - Assert.Contains("MinNumberOfChoices must be greater or equal to", validationResults[0].ErrorMessage); + Assert.Contains("must be greater or equal to MinNumberOfChoices", validationResults[0].ErrorMessage); Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); } From f100e9649f58de51b958e93692d661fb8e760762 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Wed, 5 Jun 2024 18:48:33 -0700 Subject: [PATCH 10/19] AD-99 Added support for "Selected = null" for initial state --- .../HelperTests/MerkleTreeTest.cs | 4 ++-- TrueVote.Api/Helpers/CustomValidators.cs | 23 ++++++++++++------- TrueVote.Api/Models/CandidateModel.cs | 3 +-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs b/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs index b58d461..0412521 100644 --- a/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs +++ b/TrueVote.Api.Tests/HelperTests/MerkleTreeTest.cs @@ -70,8 +70,8 @@ public void CreatesHashFromBallotObject() Assert.NotNull(hash1); Assert.NotNull(hash2); Assert.Equal(hash1, hash2); - Assert.Equal("67", hash1[0].ToString()); - Assert.Equal("67", hash2[0].ToString()); + Assert.Equal("45", hash1[0].ToString()); + Assert.Equal("45", hash2[0].ToString()); } [Fact] diff --git a/TrueVote.Api/Helpers/CustomValidators.cs b/TrueVote.Api/Helpers/CustomValidators.cs index 59f9626..8c4f6df 100644 --- a/TrueVote.Api/Helpers/CustomValidators.cs +++ b/TrueVote.Api/Helpers/CustomValidators.cs @@ -31,10 +31,17 @@ protected override ValidationResult IsValid(object value, ValidationContext vali return new ValidationResult($"Property '{CandidatesPropertyName}' is not a valid List type.", [validationContext.MemberName]); } + // If any of selections are null, then don't validate. This indicates a property check on an unfilled model. + var nullCount = ((IEnumerable) candidatePropertyValue).Cast().Where(c => c.Selected == null).Count(); + if (nullCount > 0) + { + return ValidationResult.Success; + } + // Calculate the number of selections in the candidate choices - var count = ((IEnumerable) candidatePropertyValue).Cast().Where(c => c.Selected == true).Count(); + var selectedCount = ((IEnumerable) candidatePropertyValue).Cast().Where(c => c.Selected == true).Count(); - return ValidateCount(value, count, validationContext); + return ValidateCount(value, selectedCount, validationContext); } protected abstract ValidationResult ValidateCount(object value, int count, ValidationContext validationContext); @@ -46,12 +53,12 @@ public MaxNumberOfChoicesValidatorAttribute(string propertyName) : base(property { } - protected override ValidationResult ValidateCount(object value, int count, ValidationContext validationContext) + protected override ValidationResult ValidateCount(object value, int selectedCount, ValidationContext validationContext) { var maxNumberOfChoices = value as int?; - if (maxNumberOfChoices.HasValue && (count > maxNumberOfChoices.Value)) + if (maxNumberOfChoices.HasValue && (selectedCount > maxNumberOfChoices.Value)) { - return new ValidationResult($"Number of selected items in '{CandidatesPropertyName}' cannot exceed MaxNumberOfChoices. MaxNumberOfChoices: {maxNumberOfChoices}, Count: {count}", [validationContext.MemberName]); + return new ValidationResult($"Number of selected items in '{CandidatesPropertyName}' cannot exceed MaxNumberOfChoices. MaxNumberOfChoices: {maxNumberOfChoices}, SelectedCount: {selectedCount}", [validationContext.MemberName]); } return ValidationResult.Success; @@ -64,12 +71,12 @@ public MinNumberOfChoicesValidatorAttribute(string propertyName) : base(property { } - protected override ValidationResult ValidateCount(object value, int count, ValidationContext validationContext) + protected override ValidationResult ValidateCount(object value, int selectedCount, ValidationContext validationContext) { var minNumberOfChoices = value as int?; - if (minNumberOfChoices.HasValue && (count < minNumberOfChoices.Value)) + if (minNumberOfChoices.HasValue && (selectedCount < minNumberOfChoices.Value)) { - return new ValidationResult($"Number of selected items in '{CandidatesPropertyName}' must be greater or equal to MinNumberOfChoices. MinNumberOfChoices: {minNumberOfChoices}, Count: {count}", [validationContext.MemberName]); + return new ValidationResult($"Number of selected items in '{CandidatesPropertyName}' must be greater or equal to MinNumberOfChoices. MinNumberOfChoices: {minNumberOfChoices}, Count: {selectedCount}", [validationContext.MemberName]); } return ValidationResult.Success; diff --git a/TrueVote.Api/Models/CandidateModel.cs b/TrueVote.Api/Models/CandidateModel.cs index b2a6f46..d0f1d44 100644 --- a/TrueVote.Api/Models/CandidateModel.cs +++ b/TrueVote.Api/Models/CandidateModel.cs @@ -106,7 +106,7 @@ public class CandidateModel [Description("Selected")] [JsonPropertyName("Selected")] [JsonProperty(nameof(Selected), Required = Required.Default)] - public bool Selected { get; set; } = false; + public bool? Selected { get; set; } = null; [Description("SelectedMetadata")] [MaxLength(1024)] @@ -114,6 +114,5 @@ public class CandidateModel [JsonPropertyName("SelectedMetadata")] [JsonProperty(nameof(SelectedMetadata), Required = Required.Default)] public string SelectedMetadata { get; set; } = string.Empty; - } } From bd070841132e2769780440d7be2b6cc2cf910b86 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Thu, 6 Jun 2024 19:00:54 -0700 Subject: [PATCH 11/19] AD-99 Added TODO for ballot validation --- TrueVote.Api/Services/Ballot.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/TrueVote.Api/Services/Ballot.cs b/TrueVote.Api/Services/Ballot.cs index 5bb5012..c8d00ba 100644 --- a/TrueVote.Api/Services/Ballot.cs +++ b/TrueVote.Api/Services/Ballot.cs @@ -46,6 +46,7 @@ public async Task SubmitBallot([FromBody] SubmitBallotModel bindS // 1. Must have a UserId and not have already submitted a ballot for this election // 2. Confirm the election id exists // 3. Confirm the election data for this ballot has not been altered. + // 4. Confirm none of the races have null for 'Selected'. Must be true or false. // ADD CODE FOR ABOVE ITEMS HERE var ballot = new BallotModel { Election = bindSubmitBallotModel.Election, BallotId = Guid.NewGuid().ToString(), DateCreated = UtcNowProviderFactory.GetProvider().UtcNow }; From 400634b00ce685ea15d02247bcd753a55aae3913 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Mon, 10 Jun 2024 15:40:34 -0700 Subject: [PATCH 12/19] Updated Packages --- TrueVote.Api/TrueVote.Api.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TrueVote.Api/TrueVote.Api.csproj b/TrueVote.Api/TrueVote.Api.csproj index 6fa7473..84cea5b 100644 --- a/TrueVote.Api/TrueVote.Api.csproj +++ b/TrueVote.Api/TrueVote.Api.csproj @@ -40,7 +40,7 @@ - + From 81d58324ef335f9854f304b43aba58f0c7b1654d Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Wed, 12 Jun 2024 12:49:06 -0700 Subject: [PATCH 13/19] Updated Packages --- TrueVote.Api/TrueVote.Api.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/TrueVote.Api/TrueVote.Api.csproj b/TrueVote.Api/TrueVote.Api.csproj index 84cea5b..ac0ca82 100644 --- a/TrueVote.Api/TrueVote.Api.csproj +++ b/TrueVote.Api/TrueVote.Api.csproj @@ -26,8 +26,8 @@ - - + + From 3e72b8bb5064775f5e4ed0c648f2d15699ff8893 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Mon, 17 Jun 2024 13:26:05 -0700 Subject: [PATCH 14/19] Updated Packages --- TrueVote.Api.Tests/TrueVote.Api.Tests.csproj | 2 +- TrueVote.Api/TrueVote.Api.csproj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj b/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj index b1bcd6c..0b4527a 100644 --- a/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj +++ b/TrueVote.Api.Tests/TrueVote.Api.Tests.csproj @@ -16,7 +16,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive all - + diff --git a/TrueVote.Api/TrueVote.Api.csproj b/TrueVote.Api/TrueVote.Api.csproj index ac0ca82..ba8e41e 100644 --- a/TrueVote.Api/TrueVote.Api.csproj +++ b/TrueVote.Api/TrueVote.Api.csproj @@ -38,7 +38,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + From 25bf9a6eefc3fae6bb9c3da6cc9158f3e386b4c6 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Mon, 17 Jun 2024 16:08:43 -0700 Subject: [PATCH 15/19] AD-99 Added custom validator for Ballot Submission --- .../HelperTests/CustomValidatorTest.cs | 27 +++++++- .../Helpers/ValidationHelper.cs | 14 +++-- TrueVote.Api.Tests/ServiceTests/BallotTest.cs | 62 ++++++++++++++----- TrueVote.Api/Helpers/CustomValidators.cs | 47 +++++++++++++- TrueVote.Api/Services/Ballot.cs | 14 +++++ 5 files changed, 139 insertions(+), 25 deletions(-) diff --git a/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs b/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs index a9c86e7..b410e51 100644 --- a/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs +++ b/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs @@ -8,6 +8,7 @@ using System.Text.Json.Serialization; using TrueVote.Api.Models; using System.Linq; +using Microsoft.AspNetCore.Mvc; namespace TrueVote.Api.Tests.HelperTests { @@ -122,7 +123,7 @@ public void TestsMinNumberOfChoicesFails() testModel.Candidates[1].Selected = true; Assert.Equal(2, testModel.Candidates.Where(c => c.Selected == true).Count()); - var validationResults = ValidationHelper.Validate(testModel); + var validationResults = ValidationHelper.Validate(testModel, true); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); Assert.Single(validationResults); @@ -138,7 +139,7 @@ public void TestsMaxNumberOfChoicesFails() testModel.Candidates[1].Selected = true; Assert.Equal(2, testModel.Candidates.Where(c => c.Selected == true).Count()); - var validationResults = ValidationHelper.Validate(testModel); + var validationResults = ValidationHelper.Validate(testModel, true); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); Assert.Single(validationResults); @@ -197,5 +198,27 @@ public void TestsNumberOfChoicesMinNotFoundProperty() Assert.Contains("Property not found", validationResults[0].ErrorMessage); Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); } + + [Fact] + public void ValidatesModelWithNestedModelProperties() + { + var validationResults = new List(); + var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; + var validationContext = new ValidationContext(baseBallotObj); + var validModel = RecursiveValidator.TryValidateObjectRecursive(baseBallotObj, validationContext, validationResults); + Assert.True(validModel); + Assert.Empty(validationResults); + } + + [Fact] + public void ValidatorHandlesNullModel() + { + var validationResults = new List(); + var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; + var validationContext = new ValidationContext(baseBallotObj); + var validModel = RecursiveValidator.TryValidateObjectRecursive(null, validationContext, validationResults); + Assert.True(validModel); + Assert.Empty(validationResults); + } } } diff --git a/TrueVote.Api.Tests/Helpers/ValidationHelper.cs b/TrueVote.Api.Tests/Helpers/ValidationHelper.cs index 9fa4c3b..d363718 100644 --- a/TrueVote.Api.Tests/Helpers/ValidationHelper.cs +++ b/TrueVote.Api.Tests/Helpers/ValidationHelper.cs @@ -7,19 +7,23 @@ namespace TrueVote.Api.Tests.Helpers { public static class ValidationHelper { - public static IList Validate(object model) + public static IList Validate(object model, bool isBallot = false) { var validationResults = new List(); - ValidateRecursive(model, validationResults); + ValidateRecursive(model, validationResults, isBallot); return validationResults; } - private static void ValidateRecursive(object instance, IList validationResults) + private static void ValidateRecursive(object instance, IList validationResults, bool isBallot) { if (instance == null) return; var context = new ValidationContext(instance); + if (isBallot) + { + context.Items["IsBallot"] = true; + } var results = new List(); Validator.TryValidateObject(instance, context, results, true); @@ -58,14 +62,14 @@ private static void ValidateRecursive(object instance, IList v { if (item != null) { - ValidateRecursive(item, validationResults); + ValidateRecursive(item, validationResults, isBallot); } } } } else { - ValidateRecursive(propertyValue, validationResults); + ValidateRecursive(propertyValue, validationResults, isBallot); } } } diff --git a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs index cce9702..572c12f 100644 --- a/TrueVote.Api.Tests/ServiceTests/BallotTest.cs +++ b/TrueVote.Api.Tests/ServiceTests/BallotTest.cs @@ -25,7 +25,7 @@ public BallotTest(ITestOutputHelper output) : base(output) public async Task SubmitsBallot() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; - var validationResults = ValidationHelper.Validate(baseBallotObj); + var validationResults = ValidationHelper.Validate(baseBallotObj, true); Assert.Empty(validationResults); var ret = await _ballotApi.SubmitBallot(baseBallotObj); @@ -48,69 +48,101 @@ public async Task SubmitsBallot() } [Fact] - public void SubmitsBallotWithInvalidNumberOfMaxChoices() + public async Task SubmitsBallotWithInvalidNumberOfMaxChoices() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; baseBallotObj.Election.Races[0].MaxNumberOfChoices = -1; - var validationResults = ValidationHelper.Validate(baseBallotObj); + var validationResults = ValidationHelper.Validate(baseBallotObj, true); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); // Assert.Single(validationResults); Assert.Contains("MaxNumberOfChoices must be between 0", validationResults[0].ErrorMessage); Assert.Equal("MaxNumberOfChoices", validationResults[0].MemberNames.First()); + + var ret = await _ballotApi.SubmitBallot(baseBallotObj); + Assert.NotNull(ret); + Assert.Equal(StatusCodes.Status400BadRequest, ((IStatusCodeActionResult) ret).StatusCode); + var objectResult = Assert.IsType(ret); + var validationProblemDetails = Assert.IsType(objectResult.Value); + Assert.Contains(validationProblemDetails.Errors.First().Value, v => v.Contains("MaxNumberOfChoices must be between 0")); + Assert.Contains("MaxNumberOfChoices", validationProblemDetails.Errors.Keys); } [Fact] - public void SubmitsBallotWithAboveCandidateCountNumberOfMaxChoices() + public async Task SubmitsBallotWithAboveCandidateCountNumberOfMaxChoices() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; baseBallotObj.Election.Races[0].MaxNumberOfChoices = 1; baseBallotObj.Election.Races[0].Candidates[0].Selected = true; baseBallotObj.Election.Races[0].Candidates[1].Selected = true; - var validationResults = ValidationHelper.Validate(baseBallotObj); + var validationResults = ValidationHelper.Validate(baseBallotObj, true); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); Assert.Single(validationResults); Assert.Contains("cannot exceed MaxNumberOfChoices", validationResults[0].ErrorMessage); Assert.Equal("MaxNumberOfChoices", validationResults[0].MemberNames.First()); + + var ret = await _ballotApi.SubmitBallot(baseBallotObj); + Assert.NotNull(ret); + Assert.Equal(StatusCodes.Status400BadRequest, ((IStatusCodeActionResult) ret).StatusCode); + var objectResult = Assert.IsType(ret); + var validationProblemDetails = Assert.IsType(objectResult.Value); + Assert.Contains(validationProblemDetails.Errors.First().Value, v => v.Contains("cannot exceed MaxNumberOfChoices")); + Assert.Contains("MaxNumberOfChoices", validationProblemDetails.Errors.Keys); } [Fact] - public void SubmitsBallotWithInvalidNumberOfMinChoices() + public async Task SubmitsBallotWithInvalidNumberOfMinChoices() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; baseBallotObj.Election.Races[0].MinNumberOfChoices = -1; - var validationResults = ValidationHelper.Validate(baseBallotObj); + var validationResults = ValidationHelper.Validate(baseBallotObj, true); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); Assert.Single(validationResults); Assert.Contains("MinNumberOfChoices must be between 0", validationResults[0].ErrorMessage); Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); + + var ret = await _ballotApi.SubmitBallot(baseBallotObj); + Assert.NotNull(ret); + Assert.Equal(StatusCodes.Status400BadRequest, ((IStatusCodeActionResult) ret).StatusCode); + var objectResult = Assert.IsType(ret); + var validationProblemDetails = Assert.IsType(objectResult.Value); + Assert.Contains(validationProblemDetails.Errors.First().Value, v => v.Contains("MinNumberOfChoices must be between 0")); + Assert.Contains("MinNumberOfChoices", validationProblemDetails.Errors.Keys); } [Fact] - public void SubmitsBallotWithBelowCandidateCountNumberOfMinChoices() + public async Task SubmitsBallotWithBelowCandidateCountNumberOfMinChoices() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; baseBallotObj.Election.Races[0].MinNumberOfChoices = 2; baseBallotObj.Election.Races[0].Candidates[0].Selected = true; - var validationResults = ValidationHelper.Validate(baseBallotObj); + var validationResults = ValidationHelper.Validate(baseBallotObj, true); Assert.NotEmpty(validationResults); Assert.NotNull(validationResults); Assert.Single(validationResults); Assert.Contains("must be greater or equal to MinNumberOfChoices", validationResults[0].ErrorMessage); Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); + + var ret = await _ballotApi.SubmitBallot(baseBallotObj); + Assert.NotNull(ret); + Assert.Equal(StatusCodes.Status400BadRequest, ((IStatusCodeActionResult) ret).StatusCode); + var objectResult = Assert.IsType(ret); + var validationProblemDetails = Assert.IsType(objectResult.Value); + Assert.Contains(validationProblemDetails.Errors.First().Value, v => v.Contains("must be greater or equal to MinNumberOfChoices")); + Assert.Contains("MinNumberOfChoices", validationProblemDetails.Errors.Keys); } [Fact] public async Task HandlesSubmitBallotHashingError() { var baseBallotObj = new SubmitBallotModel { Election = MoqData.MockBallotData[1].Election }; - var validationResults = ValidationHelper.Validate(baseBallotObj); + var validationResults = ValidationHelper.Validate(baseBallotObj, true); Assert.Empty(validationResults); var mockValidator = new Mock(); @@ -133,7 +165,7 @@ public async Task HandlesSubmitBallotHashingError() public async Task FindsBallot() { var findBallotObj = new FindBallotModel { BallotId = "ballotid3" }; - var validationResults = ValidationHelper.Validate(findBallotObj); + var validationResults = ValidationHelper.Validate(findBallotObj, true); Assert.Empty(validationResults); var ret = await _ballotApi.BallotFind(findBallotObj); @@ -153,7 +185,7 @@ public async Task FindsBallot() public async Task HandlesUnfoundBallot() { var findBallotObj = new FindBallotModel { BallotId = "not going to find anything" }; - var validationResults = ValidationHelper.Validate(findBallotObj); + var validationResults = ValidationHelper.Validate(findBallotObj, true); Assert.Empty(validationResults); var ret = await _ballotApi.BallotFind(findBallotObj); @@ -168,7 +200,7 @@ public async Task HandlesUnfoundBallot() public async Task CountsBallots() { var countBallotsObj = new CountBallotModel { DateCreatedStart = new DateTime(2022, 01, 01), DateCreatedEnd = new DateTime(2033, 12, 31) }; - var validationResults = ValidationHelper.Validate(countBallotsObj); + var validationResults = ValidationHelper.Validate(countBallotsObj, true); Assert.Empty(validationResults); var ret = await _ballotApi.BallotCount(countBallotsObj); @@ -187,7 +219,7 @@ public async Task CountsBallots() public async Task FindsBallotHash() { var findBallotHashObj = new FindBallotHashModel { BallotId = "ballotid1" }; - var validationResults = ValidationHelper.Validate(findBallotHashObj); + var validationResults = ValidationHelper.Validate(findBallotHashObj, true); Assert.Empty(validationResults); var ret = await _ballotApi.BallotHashFind(findBallotHashObj); @@ -207,7 +239,7 @@ public async Task FindsBallotHash() public async Task HandlesUnfoundBallotHash() { var findBallotHashObj = new FindBallotHashModel { BallotId = "not going to find anything" }; - var validationResults = ValidationHelper.Validate(findBallotHashObj); + var validationResults = ValidationHelper.Validate(findBallotHashObj, true); Assert.Empty(validationResults); var ret = await _ballotApi.BallotHashFind(findBallotHashObj); diff --git a/TrueVote.Api/Helpers/CustomValidators.cs b/TrueVote.Api/Helpers/CustomValidators.cs index 8c4f6df..e5f759d 100644 --- a/TrueVote.Api/Helpers/CustomValidators.cs +++ b/TrueVote.Api/Helpers/CustomValidators.cs @@ -1,10 +1,47 @@ #pragma warning disable IDE0046 // Convert to conditional expression using System.Collections; using System.ComponentModel.DataAnnotations; +using System.Reflection; using TrueVote.Api.Models; namespace TrueVote.Api.Helpers { + public static class RecursiveValidator + { + public static bool TryValidateObjectRecursive(object obj, ValidationContext validationContext, List results) + { + if (obj == null) return true; + + var result = Validator.TryValidateObject(obj, validationContext, results, true); + + foreach (var property in obj.GetType().GetProperties(BindingFlags.Public | BindingFlags.Instance)) + { + var value = property.GetValue(obj); + if (value == null) continue; + + // Check if the property is a collection + if (value is IEnumerable enumerable) + { + foreach (var item in enumerable) + { + if (item != null) + { + var nestedContext = new ValidationContext(item, validationContext, validationContext.Items); + result = TryValidateObjectRecursive(item, nestedContext, results) && result; + } + } + } + else if (property.PropertyType.IsClass && property.PropertyType != typeof(string)) + { + var nestedContext = new ValidationContext(value, validationContext, validationContext.Items); + result = TryValidateObjectRecursive(value, nestedContext, results) && result; + } + } + + return result; + } + } + public abstract class NumberOfChoicesValidatorAttribute : ValidationAttribute { protected readonly string CandidatesPropertyName; @@ -31,9 +68,13 @@ protected override ValidationResult IsValid(object value, ValidationContext vali return new ValidationResult($"Property '{CandidatesPropertyName}' is not a valid List type.", [validationContext.MemberName]); } - // If any of selections are null, then don't validate. This indicates a property check on an unfilled model. - var nullCount = ((IEnumerable) candidatePropertyValue).Cast().Where(c => c.Selected == null).Count(); - if (nullCount > 0) + // If not a Ballot, then no need to validate. Get out. + // This isn't the best way of doing this. It's a bit of a hack. The issue is that we want to use the Election model + // both for creating an election and ballot submission. This is optimal for model re-use. But the data annotations + // work for ballot submission, are too tight for election creation. So this flag gets around that. When creating an election + // if the "IsBallot" flag isn't set, the validator simply allows the validation to proceed. + var isBallot = validationContext.Items.Where(i => i.Key.ToString() == "IsBallot"); + if (!isBallot.Any()) { return ValidationResult.Success; } diff --git a/TrueVote.Api/Services/Ballot.cs b/TrueVote.Api/Services/Ballot.cs index c8d00ba..73bf3ef 100644 --- a/TrueVote.Api/Services/Ballot.cs +++ b/TrueVote.Api/Services/Ballot.cs @@ -1,4 +1,5 @@ using System.ComponentModel; +using System.ComponentModel.DataAnnotations; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; using TrueVote.Api.Helpers; @@ -48,6 +49,19 @@ public async Task SubmitBallot([FromBody] SubmitBallotModel bindS // 3. Confirm the election data for this ballot has not been altered. // 4. Confirm none of the races have null for 'Selected'. Must be true or false. // ADD CODE FOR ABOVE ITEMS HERE + var validationResults = new List(); + var validationContext = new ValidationContext(bindSubmitBallotModel); + validationContext.Items["IsBallot"] = true; + var validModel = RecursiveValidator.TryValidateObjectRecursive(bindSubmitBallotModel, validationContext, validationResults); + if (!validModel) + { + var errorDictionary = validationResults + .Select(vr => new KeyValuePair(vr.MemberNames.FirstOrDefault(), vr.ErrorMessage)) + .GroupBy(kvp => kvp.Key) + .ToDictionary(g => g.Key, g => g.Select(kvp => kvp.Value).ToArray()); + + return ValidationProblem(new ValidationProblemDetails(errorDictionary)); + } var ballot = new BallotModel { Election = bindSubmitBallotModel.Election, BallotId = Guid.NewGuid().ToString(), DateCreated = UtcNowProviderFactory.GetProvider().UtcNow }; await _trueVoteDbContext.EnsureCreatedAsync(); From 819bc98724000d982337809fed6f6461ac2059d4 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Mon, 17 Jun 2024 18:18:43 -0700 Subject: [PATCH 16/19] AD-99 Simplified dictionary aggregation of error array returned. Added the Name property of the Race - useful in returning error messages. --- TrueVote.Api/Helpers/CustomValidators.cs | 29 +++++++++++++++++++----- TrueVote.Api/Models/RaceModel.cs | 4 ++-- TrueVote.Api/Services/Ballot.cs | 8 ++----- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/TrueVote.Api/Helpers/CustomValidators.cs b/TrueVote.Api/Helpers/CustomValidators.cs index e5f759d..3ac8420 100644 --- a/TrueVote.Api/Helpers/CustomValidators.cs +++ b/TrueVote.Api/Helpers/CustomValidators.cs @@ -11,7 +11,6 @@ public static class RecursiveValidator public static bool TryValidateObjectRecursive(object obj, ValidationContext validationContext, List results) { if (obj == null) return true; - var result = Validator.TryValidateObject(obj, validationContext, results, true); foreach (var property in obj.GetType().GetProperties(BindingFlags.Public | BindingFlags.Instance)) @@ -40,15 +39,26 @@ public static bool TryValidateObjectRecursive(object obj, ValidationContext vali return result; } + + public static Dictionary GetValidationErrorsDictionary(List results) + { + return results + .SelectMany(vr => vr.MemberNames.Select(memberName => new { memberName, ErrorMessage = vr.ErrorMessage ?? string.Empty })) + .GroupBy(x => x.memberName, x => x.ErrorMessage) + .ToDictionary(g => g.Key, g => g.ToArray()); + } } public abstract class NumberOfChoicesValidatorAttribute : ValidationAttribute { protected readonly string CandidatesPropertyName; + protected readonly string RacePropertyName; + protected string RacePropertyValue = string.Empty; - protected NumberOfChoicesValidatorAttribute(string propertyName) + protected NumberOfChoicesValidatorAttribute(string propertyName, string racePropertyName) { CandidatesPropertyName = propertyName; + RacePropertyName = racePropertyName; } protected override ValidationResult IsValid(object value, ValidationContext validationContext) @@ -68,6 +78,13 @@ protected override ValidationResult IsValid(object value, ValidationContext vali return new ValidationResult($"Property '{CandidatesPropertyName}' is not a valid List type.", [validationContext.MemberName]); } + // Get the value of the Name Property + var nameProperty = validationContext.ObjectType.GetProperty(RacePropertyName); + if (nameProperty != null) + { + RacePropertyValue = nameProperty.GetValue(validationContext.ObjectInstance).ToString(); + } + // If not a Ballot, then no need to validate. Get out. // This isn't the best way of doing this. It's a bit of a hack. The issue is that we want to use the Election model // both for creating an election and ballot submission. This is optimal for model re-use. But the data annotations @@ -90,7 +107,7 @@ protected override ValidationResult IsValid(object value, ValidationContext vali public class MaxNumberOfChoicesValidatorAttribute : NumberOfChoicesValidatorAttribute { - public MaxNumberOfChoicesValidatorAttribute(string propertyName) : base(propertyName) + public MaxNumberOfChoicesValidatorAttribute(string propertyName, string racePropertyName) : base(propertyName, racePropertyName) { } @@ -99,7 +116,7 @@ protected override ValidationResult ValidateCount(object value, int selectedCoun var maxNumberOfChoices = value as int?; if (maxNumberOfChoices.HasValue && (selectedCount > maxNumberOfChoices.Value)) { - return new ValidationResult($"Number of selected items in '{CandidatesPropertyName}' cannot exceed MaxNumberOfChoices. MaxNumberOfChoices: {maxNumberOfChoices}, SelectedCount: {selectedCount}", [validationContext.MemberName]); + return new ValidationResult($"Number of selected items in '{CandidatesPropertyName}' cannot exceed MaxNumberOfChoices for '{RacePropertyValue}'. MaxNumberOfChoices: {maxNumberOfChoices}, SelectedCount: {selectedCount}", [validationContext.MemberName]); } return ValidationResult.Success; @@ -108,7 +125,7 @@ protected override ValidationResult ValidateCount(object value, int selectedCoun public class MinNumberOfChoicesValidatorAttribute : NumberOfChoicesValidatorAttribute { - public MinNumberOfChoicesValidatorAttribute(string propertyName) : base(propertyName) + public MinNumberOfChoicesValidatorAttribute(string propertyName, string racePropertyName) : base(propertyName, racePropertyName) { } @@ -117,7 +134,7 @@ protected override ValidationResult ValidateCount(object value, int selectedCoun var minNumberOfChoices = value as int?; if (minNumberOfChoices.HasValue && (selectedCount < minNumberOfChoices.Value)) { - return new ValidationResult($"Number of selected items in '{CandidatesPropertyName}' must be greater or equal to MinNumberOfChoices. MinNumberOfChoices: {minNumberOfChoices}, Count: {selectedCount}", [validationContext.MemberName]); + return new ValidationResult($"Number of selected items in '{CandidatesPropertyName}' must be greater or equal to MinNumberOfChoices for '{RacePropertyValue}'. MinNumberOfChoices: {minNumberOfChoices}, Count: {selectedCount}", [validationContext.MemberName]); } return ValidationResult.Success; diff --git a/TrueVote.Api/Models/RaceModel.cs b/TrueVote.Api/Models/RaceModel.cs index 96c4ad5..097a1c5 100644 --- a/TrueVote.Api/Models/RaceModel.cs +++ b/TrueVote.Api/Models/RaceModel.cs @@ -100,7 +100,7 @@ public class RaceModel [Range(0, int.MaxValue)] [JsonPropertyName("MaxNumberOfChoices")] [JsonProperty(nameof(MaxNumberOfChoices), Required = Required.Default)] - [MaxNumberOfChoicesValidator(nameof(Candidates))] + [MaxNumberOfChoicesValidator(nameof(Candidates), nameof(Name))] public int? MaxNumberOfChoices { get; set; } [Description("Min Number of Choices")] @@ -108,7 +108,7 @@ public class RaceModel [Range(0, int.MaxValue)] [JsonPropertyName("MinNumberOfChoices")] [JsonProperty(nameof(MinNumberOfChoices), Required = Required.Default)] - [MinNumberOfChoicesValidator(nameof(Candidates))] + [MinNumberOfChoicesValidator(nameof(Candidates), nameof(Name))] public int? MinNumberOfChoices { get; set; } [Required] diff --git a/TrueVote.Api/Services/Ballot.cs b/TrueVote.Api/Services/Ballot.cs index 73bf3ef..bdcf230 100644 --- a/TrueVote.Api/Services/Ballot.cs +++ b/TrueVote.Api/Services/Ballot.cs @@ -52,13 +52,9 @@ public async Task SubmitBallot([FromBody] SubmitBallotModel bindS var validationResults = new List(); var validationContext = new ValidationContext(bindSubmitBallotModel); validationContext.Items["IsBallot"] = true; - var validModel = RecursiveValidator.TryValidateObjectRecursive(bindSubmitBallotModel, validationContext, validationResults); - if (!validModel) + if (!RecursiveValidator.TryValidateObjectRecursive(bindSubmitBallotModel, validationContext, validationResults)) { - var errorDictionary = validationResults - .Select(vr => new KeyValuePair(vr.MemberNames.FirstOrDefault(), vr.ErrorMessage)) - .GroupBy(kvp => kvp.Key) - .ToDictionary(g => g.Key, g => g.Select(kvp => kvp.Value).ToArray()); + var errorDictionary = RecursiveValidator.GetValidationErrorsDictionary(validationResults); return ValidationProblem(new ValidationProblemDetails(errorDictionary)); } From 7ad5225d78a8ad734227865cd5a97241e83e57c3 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Mon, 17 Jun 2024 18:53:59 -0700 Subject: [PATCH 17/19] AD-99 Improved tests to cover multiple errors in returned validation results --- .../HelperTests/CustomValidatorTest.cs | 88 +++++++++++++++++-- TrueVote.Api/Services/Ballot.cs | 1 + 2 files changed, 81 insertions(+), 8 deletions(-) diff --git a/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs b/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs index b410e51..bc58c63 100644 --- a/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs +++ b/TrueVote.Api.Tests/HelperTests/CustomValidatorTest.cs @@ -20,16 +20,23 @@ public class CandidateTestModel [JsonProperty(nameof(Candidates), Required = Required.Default)] public List Candidates { get; set; } = new List(); + [Description("Name")] + [MaxLength(2048)] + [DataType(DataType.Text)] + [JsonPropertyName("Name")] + [JsonProperty(nameof(Name), Required = Required.Default)] + public string Name { get; set; } = string.Empty; + [Description("Max Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [MaxNumberOfChoicesValidator(nameof(Candidates))] + [MaxNumberOfChoicesValidator(nameof(Candidates), nameof(Name))] public int? MaxNumberOfChoices { get; set; } [Description("Min Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [MinNumberOfChoicesValidator(nameof(Candidates))] + [MinNumberOfChoicesValidator(nameof(Candidates), nameof(Name))] public int? MinNumberOfChoices { get; set; } } @@ -41,16 +48,23 @@ public class CandidateTestModelBlankProperty [JsonProperty(nameof(Candidates), Required = Required.Default)] public List Candidates { get; set; } = new List(); + [Description("Name")] + [MaxLength(2048)] + [DataType(DataType.Text)] + [JsonPropertyName("Name")] + [JsonProperty(nameof(Name), Required = Required.Default)] + public string Name { get; set; } = string.Empty; + [Description("Max Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [MaxNumberOfChoicesValidator("")] + [MaxNumberOfChoicesValidator("", "")] public int? MaxNumberOfChoices { get; set; } [Description("Min Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [MinNumberOfChoicesValidator("")] + [MinNumberOfChoicesValidator("", "")] public int? MinNumberOfChoices { get; set; } } @@ -62,10 +76,17 @@ public class CandidateTestModelMinInvalidProperty [JsonProperty(nameof(Candidates), Required = Required.Default)] public string Candidates { get; set; } + [Description("Name")] + [MaxLength(2048)] + [DataType(DataType.Text)] + [JsonPropertyName("Name")] + [JsonProperty(nameof(Name), Required = Required.Always)] + public string Name { get; set; } = string.Empty; + [Description("Min Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [MinNumberOfChoicesValidator(nameof(Candidates))] + [MinNumberOfChoicesValidator(nameof(Candidates), nameof(Name))] public int? MinNumberOfChoices { get; set; } } @@ -77,10 +98,17 @@ public class CandidateTestModelMaxInvalidProperty [JsonProperty(nameof(Candidates), Required = Required.Default)] public string Candidates { get; set; } + [Description("Name")] + [MaxLength(2048)] + [DataType(DataType.Text)] + [JsonPropertyName("Name")] + [JsonProperty(nameof(Name), Required = Required.Always)] + public string Name { get; set; } = string.Empty; + [Description("Max Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [MaxNumberOfChoicesValidator(nameof(Candidates))] + [MaxNumberOfChoicesValidator(nameof(Candidates), nameof(Name))] public int? MaxNumberOfChoices { get; set; } } @@ -89,7 +117,7 @@ public class CandidateTestModelMaxNotFoundProperty [Description("Max Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [MaxNumberOfChoicesValidator("foo")] + [MaxNumberOfChoicesValidator("foo", "bar")] public int? MaxNumberOfChoices { get; set; } } @@ -98,7 +126,7 @@ public class CandidateTestModelMinNotFoundProperty [Description("Min Number of Choices")] [DataType("integer")] [Range(0, int.MaxValue)] - [MaxNumberOfChoicesValidator("foo")] + [MaxNumberOfChoicesValidator("foo", "bar")] public int? MinNumberOfChoices { get; set; } } @@ -210,6 +238,46 @@ public void ValidatesModelWithNestedModelProperties() Assert.Empty(validationResults); } + [Fact] + public void ValidatesMinNumberOfChoicesInvalidProperty() + { + var validationResults = new List(); + var testModel = new CandidateTestModelMinInvalidProperty { MinNumberOfChoices = 3, Candidates = "foo" }; + var validationContext = new ValidationContext(testModel); + var validModel = RecursiveValidator.TryValidateObjectRecursive(testModel, validationContext, validationResults); + Assert.False(validModel); + Assert.NotEmpty(validationResults); + Assert.NotNull(validationResults); + Assert.Single(validationResults); + Assert.Contains("Property 'Candidates' is not a valid List type", validationResults[0].ErrorMessage); + Assert.Equal("MinNumberOfChoices", validationResults[0].MemberNames.First()); + + var errorDictionary = RecursiveValidator.GetValidationErrorsDictionary(validationResults); + Assert.NotEmpty(errorDictionary); + Assert.NotNull(errorDictionary); + Assert.Single(errorDictionary); + } + + [Fact] + public void GetValidationErrorsDictionaryShouldHandleMissingErrorMessage() + { + var validationResults = new List + { + new("Error 1", ["Property1"]), + new("Error 2", ["Property2"]), + new(null, ["Property3"]) + }; + + var errorDictionary = RecursiveValidator.GetValidationErrorsDictionary(validationResults); + + Assert.NotEmpty(errorDictionary); + Assert.NotNull(errorDictionary); + Assert.Equal(3, errorDictionary.Count); + Assert.Equal(expected: ["Error 1"], errorDictionary["Property1"]); + Assert.Equal(expected: ["Error 2"], errorDictionary["Property2"]); + Assert.Equal(expected: [string.Empty], errorDictionary["Property3"]); + } + [Fact] public void ValidatorHandlesNullModel() { @@ -219,6 +287,10 @@ public void ValidatorHandlesNullModel() var validModel = RecursiveValidator.TryValidateObjectRecursive(null, validationContext, validationResults); Assert.True(validModel); Assert.Empty(validationResults); + + var errorDictionary = RecursiveValidator.GetValidationErrorsDictionary(validationResults); + Assert.Empty(errorDictionary); + Assert.NotNull(errorDictionary); } } } diff --git a/TrueVote.Api/Services/Ballot.cs b/TrueVote.Api/Services/Ballot.cs index bdcf230..048c757 100644 --- a/TrueVote.Api/Services/Ballot.cs +++ b/TrueVote.Api/Services/Ballot.cs @@ -49,6 +49,7 @@ public async Task SubmitBallot([FromBody] SubmitBallotModel bindS // 3. Confirm the election data for this ballot has not been altered. // 4. Confirm none of the races have null for 'Selected'. Must be true or false. // ADD CODE FOR ABOVE ITEMS HERE + var validationResults = new List(); var validationContext = new ValidationContext(bindSubmitBallotModel); validationContext.Items["IsBallot"] = true; From 031e53510586ba1db3e83cede4c8ddc9eb934c01 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Tue, 18 Jun 2024 07:16:30 -0700 Subject: [PATCH 18/19] Updated Packages --- TrueVote.Api/TrueVote.Api.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/TrueVote.Api/TrueVote.Api.csproj b/TrueVote.Api/TrueVote.Api.csproj index ba8e41e..6535334 100644 --- a/TrueVote.Api/TrueVote.Api.csproj +++ b/TrueVote.Api/TrueVote.Api.csproj @@ -45,7 +45,7 @@ - + all @@ -54,7 +54,7 @@ - + From 6150c65d17880cff501761949b7d5e9421766ae7 Mon Sep 17 00:00:00 2001 From: Brett Morrison Date: Tue, 18 Jun 2024 14:53:06 -0700 Subject: [PATCH 19/19] AD-99 Added comment reference to DTO ticket --- TrueVote.Api/Services/Ballot.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TrueVote.Api/Services/Ballot.cs b/TrueVote.Api/Services/Ballot.cs index 048c757..df5c612 100644 --- a/TrueVote.Api/Services/Ballot.cs +++ b/TrueVote.Api/Services/Ballot.cs @@ -52,7 +52,7 @@ public async Task SubmitBallot([FromBody] SubmitBallotModel bindS var validationResults = new List(); var validationContext = new ValidationContext(bindSubmitBallotModel); - validationContext.Items["IsBallot"] = true; + validationContext.Items["IsBallot"] = true; // TODO https://truevote.atlassian.net/browse/AD-113 if (!RecursiveValidator.TryValidateObjectRecursive(bindSubmitBallotModel, validationContext, validationResults)) { var errorDictionary = RecursiveValidator.GetValidationErrorsDictionary(validationResults);