server icon indicating copy to clipboard operation
server copied to clipboard

[PM-3581] Fix Postgres Time

Open justindbaur opened this issue 2 years ago • 1 comments
trafficstars

Type of change

- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

PostgreSQL time has been weird since upgrading to .NET 6. Before it would read a date like such: 2023-08-31T16:29:08.360081+00:00 and with the previous converter would read it as 12:29:08.3600810 (DateTime.TimeOfDay) it would still show it as UTC though. This means a date created in C# can be compared to a date read from the database and converted into a C# object. Now that it could be done I could make tests that do these checks across databases but SQL Server actually had less precision so the times couldn't compare perfectly. An example of that data is below:

SQL Server
C#: 16:20:30.2837920
SQL: 16:20:30.2833333

Postgres (Before Change)
C#: 16:29:08.3600810
SQL: 12:29:08.3600810

In DB: 2023-08-31T16:29:08.360081+00:00

Postgres (After Change)
C#: 16:22:17.4456610
SQL: 16:22:17.4456610

MySQL
C#: 16:23:50.4664000
SQL: 16:23:50.4664000

Sqlite
C#: 16:24:22.9868980
SQL: 16:24:22.9868980

Code changes

  • src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs: I changed up how time is read from the postgres database, this makes it so that it will actually be (roughly) equal the C# version without having to do a roundtrip to test that it actually changes.
  • test/Infrastructure.IntegrationTest/Comparers/LaxDateTimeComparer.cs: Tool for comparing DateTime's that have been saved to a DB. Not all of our databases have the exact same precision so this is useful for still comparing that dates are pretty equal. It does it down the the seconds instead of milliseconds or ticks. All of our databases to care some millisecond precision but not as much as C# so we'd have to round which I don't think is worth it. Seconds precision is generally enough for our application.
  • test/Infrastructure.IntegrationTest/Tools/SendRepositoryTests.cs: Add send repository tests as it's an easier place to test time since it's one of our fewer places that we allow users to set the time.
  • EFIntegrationTest: Delete tests that have now been migrated over.

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • If making database changes - make sure you also update Entity Framework queries and/or migrations
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

justindbaur avatar Aug 22 '23 17:08 justindbaur

Logo Checkmarx One – Scan Summary & Details8f13db89-2428-4cb0-9ad4-90b57b93bc32

No New Or Fixed Issues Found

bitwarden-bot avatar Aug 22 '23 17:08 bitwarden-bot

Codecov Report

Attention: Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 41.02%. Comparing base (2657585) to head (9181f4a).

Files Patch % Lines
...re.EntityFramework/Repositories/DatabaseContext.cs 50.00% 5 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3221      +/-   ##
==========================================
- Coverage   41.02%   41.02%   -0.01%     
==========================================
  Files        1247     1247              
  Lines       59908    59916       +8     
  Branches     5485     5486       +1     
==========================================
+ Hits        24580    24582       +2     
- Misses      34194    34199       +5     
- Partials     1134     1135       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jun 26 '24 14:06 codecov[bot]