From 1ec2aae72392562cee5afd6b8fb55e90b4aad71e Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 28 Jun 2024 10:13:02 -0400 Subject: [PATCH] [PM-3581] Fix Postgres Time (#3221) * Fix Postgres Time - Migrate Send Tests - Delete Old Tests * Formatting * Update Comment * Change LaxComparer to Compare Some Milliseconds * Update Comment --- .../Repositories/DatabaseContext.cs | 19 +++-- .../Tools/AutoFixture/SendFixtures.cs | 71 ------------------- .../EqualityComparers/SendCompare.cs | 26 ------- .../Tools/Repositories/SendRepositoryTests.cs | 68 ------------------ .../Comparers/LaxDateTimeComparer.cs | 34 +++++++++ .../Tools/SendRepositoryTests.cs | 66 +++++++++++++++++ 6 files changed, 115 insertions(+), 169 deletions(-) delete mode 100644 test/Infrastructure.EFIntegration.Test/Tools/AutoFixture/SendFixtures.cs delete mode 100644 test/Infrastructure.EFIntegration.Test/Tools/Repositories/EqualityComparers/SendCompare.cs delete mode 100644 test/Infrastructure.EFIntegration.Test/Tools/Repositories/SendRepositoryTests.cs create mode 100644 test/Infrastructure.IntegrationTest/Comparers/LaxDateTimeComparer.cs create mode 100644 test/Infrastructure.IntegrationTest/Tools/SendRepositoryTests.cs diff --git a/src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs b/src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs index 35e5ebdb0..8712e0c17 100644 --- a/src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs +++ b/src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs @@ -159,6 +159,20 @@ public class DatabaseContext : DbContext // Make sure this is called after configuring all the entities as it iterates through all setup entities. private void ConfigureDateTimeUtcQueries(ModelBuilder builder) { + ValueConverter converter; + if (Database.IsNpgsql()) + { + converter = new ValueConverter( + v => v, + d => new DateTimeOffset(d).UtcDateTime); + } + else + { + converter = new ValueConverter( + v => v, + v => new DateTime(v.Ticks, DateTimeKind.Utc)); + } + foreach (var entityType in builder.Model.GetEntityTypes()) { if (entityType.IsKeyless) @@ -169,10 +183,7 @@ public class DatabaseContext : DbContext { if (property.ClrType == typeof(DateTime) || property.ClrType == typeof(DateTime?)) { - property.SetValueConverter( - new ValueConverter( - v => v, - v => new DateTime(v.Ticks, DateTimeKind.Utc))); + property.SetValueConverter(converter); } } } diff --git a/test/Infrastructure.EFIntegration.Test/Tools/AutoFixture/SendFixtures.cs b/test/Infrastructure.EFIntegration.Test/Tools/AutoFixture/SendFixtures.cs deleted file mode 100644 index b4e4710d7..000000000 --- a/test/Infrastructure.EFIntegration.Test/Tools/AutoFixture/SendFixtures.cs +++ /dev/null @@ -1,71 +0,0 @@ -using AutoFixture; -using AutoFixture.Kernel; -using Bit.Core.Test.AutoFixture.UserFixtures; -using Bit.Core.Tools.Entities; -using Bit.Infrastructure.EFIntegration.Test.AutoFixture; -using Bit.Infrastructure.EFIntegration.Test.AutoFixture.Relays; -using Bit.Infrastructure.EntityFramework.Repositories; -using Bit.Infrastructure.EntityFramework.Tools.Repositories; -using Bit.Test.Common.AutoFixture; -using Bit.Test.Common.AutoFixture.Attributes; - -namespace Bit.Infrastructure.EFIntegration.Test.Tools.AutoFixture; - -internal class SendBuilder : ISpecimenBuilder -{ - public bool OrganizationOwned { get; set; } - public object Create(object request, ISpecimenContext context) - { - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } - - var type = request as Type; - if (type == null || type != typeof(Send)) - { - return new NoSpecimen(); - } - - var fixture = new Fixture(); - fixture.Customizations.Insert(0, new MaxLengthStringRelay()); - if (!OrganizationOwned) - { - fixture.Customize(composer => composer - .Without(c => c.OrganizationId)); - } - var obj = fixture.WithAutoNSubstitutions().Create(); - return obj; - } -} - -internal class EfSend : ICustomization -{ - public bool OrganizationOwned { get; set; } - public void Customize(IFixture fixture) - { - fixture.Customizations.Add(new IgnoreVirtualMembersCustomization()); - fixture.Customizations.Add(new GlobalSettingsBuilder()); - fixture.Customizations.Add(new SendBuilder()); - fixture.Customizations.Add(new UserBuilder()); - fixture.Customizations.Add(new OrganizationBuilder()); - fixture.Customizations.Add(new EfRepositoryListBuilder()); - fixture.Customizations.Add(new EfRepositoryListBuilder()); - fixture.Customizations.Add(new EfRepositoryListBuilder()); - } -} - -internal class EfUserSendAutoDataAttribute : CustomAutoDataAttribute -{ - public EfUserSendAutoDataAttribute() : base(new SutProviderCustomization(), new EfSend()) - { } -} - -internal class EfOrganizationSendAutoDataAttribute : CustomAutoDataAttribute -{ - public EfOrganizationSendAutoDataAttribute() : base(new SutProviderCustomization(), new EfSend() - { - OrganizationOwned = true, - }) - { } -} diff --git a/test/Infrastructure.EFIntegration.Test/Tools/Repositories/EqualityComparers/SendCompare.cs b/test/Infrastructure.EFIntegration.Test/Tools/Repositories/EqualityComparers/SendCompare.cs deleted file mode 100644 index bc10b6e5b..000000000 --- a/test/Infrastructure.EFIntegration.Test/Tools/Repositories/EqualityComparers/SendCompare.cs +++ /dev/null @@ -1,26 +0,0 @@ -using System.Diagnostics.CodeAnalysis; -using Bit.Core.Tools.Entities; - -namespace Bit.Infrastructure.EFIntegration.Test.Tools.Repositories.EqualityComparers; - -public class SendCompare : IEqualityComparer -{ - public bool Equals(Send x, Send y) - { - return x.Type == y.Type && - x.Data == y.Data && - x.Key == y.Key && - x.Password == y.Password && - x.MaxAccessCount == y.MaxAccessCount && - x.AccessCount == y.AccessCount && - x.ExpirationDate?.ToShortDateString() == y.ExpirationDate?.ToShortDateString() && - x.DeletionDate.ToShortDateString() == y.DeletionDate.ToShortDateString() && - x.Disabled == y.Disabled && - x.HideEmail == y.HideEmail; - } - - public int GetHashCode([DisallowNull] Send obj) - { - return base.GetHashCode(); - } -} diff --git a/test/Infrastructure.EFIntegration.Test/Tools/Repositories/SendRepositoryTests.cs b/test/Infrastructure.EFIntegration.Test/Tools/Repositories/SendRepositoryTests.cs deleted file mode 100644 index ae03d639f..000000000 --- a/test/Infrastructure.EFIntegration.Test/Tools/Repositories/SendRepositoryTests.cs +++ /dev/null @@ -1,68 +0,0 @@ -using Bit.Core.AdminConsole.Entities; -using Bit.Core.Entities; -using Bit.Core.Test.AutoFixture.Attributes; -using Bit.Core.Tools.Entities; -using Bit.Infrastructure.EFIntegration.Test.Tools.AutoFixture; -using Bit.Infrastructure.EFIntegration.Test.Tools.Repositories.EqualityComparers; -using Xunit; -using EfRepo = Bit.Infrastructure.EntityFramework.Repositories; -using EfSendRepo = Bit.Infrastructure.EntityFramework.Tools.Repositories; -using SqlRepo = Bit.Infrastructure.Dapper.Repositories; -using SqlSendRepo = Bit.Infrastructure.Dapper.Tools.Repositories; - -namespace Bit.Infrastructure.EFIntegration.Test.Tools.Repositories; - -public class SendRepositoryTests -{ - [CiSkippedTheory, EfUserSendAutoData, EfOrganizationSendAutoData] - public async Task CreateAsync_Works_DataMatches( - Send send, - User user, - Organization org, - SendCompare equalityComparer, - List suts, - List efUserRepos, - List efOrgRepos, - SqlSendRepo.SendRepository sqlSendRepo, - SqlRepo.UserRepository sqlUserRepo, - SqlRepo.OrganizationRepository sqlOrgRepo - ) - { - var savedSends = new List(); - foreach (var sut in suts) - { - var i = suts.IndexOf(sut); - - if (send.OrganizationId.HasValue) - { - var efOrg = await efOrgRepos[i].CreateAsync(org); - sut.ClearChangeTracking(); - send.OrganizationId = efOrg.Id; - } - var efUser = await efUserRepos[i].CreateAsync(user); - sut.ClearChangeTracking(); - - send.UserId = efUser.Id; - var postEfSend = await sut.CreateAsync(send); - sut.ClearChangeTracking(); - - var savedSend = await sut.GetByIdAsync(postEfSend.Id); - savedSends.Add(savedSend); - } - - var sqlUser = await sqlUserRepo.CreateAsync(user); - if (send.OrganizationId.HasValue) - { - var sqlOrg = await sqlOrgRepo.CreateAsync(org); - send.OrganizationId = sqlOrg.Id; - } - - send.UserId = sqlUser.Id; - var sqlSend = await sqlSendRepo.CreateAsync(send); - var savedSqlSend = await sqlSendRepo.GetByIdAsync(sqlSend.Id); - savedSends.Add(savedSqlSend); - - var distinctItems = savedSends.Distinct(equalityComparer); - Assert.True(!distinctItems.Skip(1).Any()); - } -} diff --git a/test/Infrastructure.IntegrationTest/Comparers/LaxDateTimeComparer.cs b/test/Infrastructure.IntegrationTest/Comparers/LaxDateTimeComparer.cs new file mode 100644 index 000000000..acff8168d --- /dev/null +++ b/test/Infrastructure.IntegrationTest/Comparers/LaxDateTimeComparer.cs @@ -0,0 +1,34 @@ +using System.Diagnostics.CodeAnalysis; + +namespace Bit.Infrastructure.IntegrationTest.Comparers; + +/// +/// A datetime comparer that doesn't care about overall ticks and instead allows a configurable allowed difference. +/// +public class LaxDateTimeComparer : IEqualityComparer +{ + public static readonly IEqualityComparer Default = new LaxDateTimeComparer(TimeSpan.FromMilliseconds(2)); + private readonly TimeSpan _allowedDifference; + + public LaxDateTimeComparer(TimeSpan allowedDifference) + { + _allowedDifference = allowedDifference; + } + + public bool Equals(DateTime x, DateTime y) + { + var difference = x - y; + return difference.Duration() < _allowedDifference; + } + + public int GetHashCode([DisallowNull] DateTime obj) + { + // Not used when used for Assert.Equal() overload + throw new NotImplementedException(); + } + + public static int RoundMilliseconds(int milliseconds) + { + return (int)Math.Round(milliseconds / 100d) * 100; + } +} diff --git a/test/Infrastructure.IntegrationTest/Tools/SendRepositoryTests.cs b/test/Infrastructure.IntegrationTest/Tools/SendRepositoryTests.cs new file mode 100644 index 000000000..0abd0e5ec --- /dev/null +++ b/test/Infrastructure.IntegrationTest/Tools/SendRepositoryTests.cs @@ -0,0 +1,66 @@ +using Bit.Core.Tools.Entities; +using Bit.Core.Tools.Enums; +using Bit.Core.Tools.Repositories; +using Bit.Infrastructure.IntegrationTest.Comparers; +using Xunit; + +namespace Bit.Infrastructure.IntegrationTest.Tools; + +public class SendRepositoryTests +{ + [DatabaseTheory, DatabaseData] + public async Task CreateAsync_Works(ISendRepository sendRepository) + { + var expirationDate = DateTime.UtcNow.AddDays(7); + + var createdSend = await sendRepository.CreateAsync(new Send + { + Data = "{\"Text\": \"2.t|t|t\"}", // TODO: EF Should enforce this + Type = SendType.Text, + AccessCount = 0, + Key = "2.t|t|t", // TODO: EF should enforce this + ExpirationDate = expirationDate, + DeletionDate = expirationDate.AddDays(7), + }); + + Assert.NotNull(createdSend.ExpirationDate); + Assert.Equal(expirationDate, createdSend.ExpirationDate!.Value, LaxDateTimeComparer.Default); + + var sendFromDatabase = await sendRepository.GetByIdAsync(createdSend.Id); + Assert.Equal(expirationDate, sendFromDatabase.ExpirationDate!.Value, LaxDateTimeComparer.Default); + Assert.Equal(SendType.Text, sendFromDatabase.Type); + Assert.Equal(0, sendFromDatabase.AccessCount); + Assert.Equal("2.t|t|t", sendFromDatabase.Key); + Assert.Equal(expirationDate.AddDays(7), sendFromDatabase.DeletionDate, LaxDateTimeComparer.Default); + Assert.Equal("{\"Text\": \"2.t|t|t\"}", sendFromDatabase.Data); + } + + [DatabaseTheory, DatabaseData] + // This test runs best on a fresh database and may fail on subsequent runs with other tests. + public async Task GetByDeletionDateAsync_Works(ISendRepository sendRepository) + { + var deletionDate = DateTime.UtcNow.AddYears(-1); + + var shouldDeleteSend = await sendRepository.CreateAsync(new Send + { + Data = "{\"Text\": \"2.t|t|t\"}", // TODO: EF Should enforce this + Type = SendType.Text, + AccessCount = 0, + Key = "2.t|t|t", // TODO: EF should enforce this + DeletionDate = deletionDate.AddSeconds(-2), + }); + + var shouldKeepSend = await sendRepository.CreateAsync(new Send + { + Data = "{\"Text\": \"2.t|t|t\"}", // TODO: EF Should enforce this + Type = SendType.Text, + AccessCount = 0, + Key = "2.t|t|t", // TODO: EF should enforce this + DeletionDate = deletionDate.AddSeconds(2), + }); + + var toDeleteSends = await sendRepository.GetManyByDeletionDateAsync(deletionDate); + var toDeleteSend = Assert.Single(toDeleteSends); + Assert.Equal(shouldDeleteSend.Id, toDeleteSend.Id); + } +}