From 4a823d5c91c4f1714797d055be4538c80ad36b8d Mon Sep 17 00:00:00 2001
From: Jason McIntosh <jason.mcintosh@harness.io>
Date: Thu, 14 Nov 2024 20:53:52 -0600
Subject: [PATCH] fix(sql): Document the changes

---
 .../persistence/ArtifactRepositoryTests.kt    |  3 +++
 .../serialization/PrecisionSqlSerializer.java | 22 +++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/keel-core-test/src/main/kotlin/com/netflix/spinnaker/keel/persistence/ArtifactRepositoryTests.kt b/keel-core-test/src/main/kotlin/com/netflix/spinnaker/keel/persistence/ArtifactRepositoryTests.kt
index 897ee3de7..0ed764fce 100644
--- a/keel-core-test/src/main/kotlin/com/netflix/spinnaker/keel/persistence/ArtifactRepositoryTests.kt
+++ b/keel-core-test/src/main/kotlin/com/netflix/spinnaker/keel/persistence/ArtifactRepositoryTests.kt
@@ -650,6 +650,9 @@ abstract class ArtifactRepositoryTests<T : ArtifactRepository> : JUnit5Minutests
     }
 
     context("artifact creation timestamp exists") {
+      // We truncate this since we're using a serialization to java that reduces the level of precision
+      // and later comparisons break otherwise.  This is needed to work with generated columns in
+      // certain databases.  See the PrecisionSqlSerializer class for more info
       val createdAt = Instant.now().truncatedTo(ChronoUnit.MILLIS)
 
       before {
diff --git a/keel-core/src/main/java/com/netflix/spinnaker/keel/serialization/PrecisionSqlSerializer.java b/keel-core/src/main/java/com/netflix/spinnaker/keel/serialization/PrecisionSqlSerializer.java
index 07772b413..839320652 100644
--- a/keel-core/src/main/java/com/netflix/spinnaker/keel/serialization/PrecisionSqlSerializer.java
+++ b/keel-core/src/main/java/com/netflix/spinnaker/keel/serialization/PrecisionSqlSerializer.java
@@ -6,6 +6,28 @@
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeFormatterBuilder;
 
+/**
+ * This class overloads the default Serialization instance in the JavaTimeModule to a precision
+ * level that the database can process via str_to_date string conversion It's needed because we do
+ * JSON serialization to a column and then the column gets used to generate another field via string
+ * conversion. Specifically * triggered_at * dismissed_at are "generated" columns. NOTE this should
+ * ONLY impact running on Linux as the system clock on Linux systems returns a higher precision in
+ * Java 17+. It's also impactful as ISO_INSTANT has NO precision limitation giving inconsistent
+ * string output as a result
+ *
+ * <p>See
+ *
+ * <ul>
+ *   <li>https://stackoverflow.com/questions/74781495/changes-in-instant-now-between-java-11-and-java-17-in-aws-ubuntu-standard-6-0
+ *   <li>https://stackoverflow.com/a/38042457 for more information.
+ * </ul>
+ *
+ * <p>NOTE we're going to 3 digits, but could go to 6 if but NO larger than 6 due to <a
+ * href="https://www.w3schools.com/sql/func_mysql_str_to_date.asp">Mysql str_to_date function
+ * limits</a> on microseconds parsing. This is used in the SQL definition: <code>
+ *  add column triggered_at datetime(3) generated always as (str_to_date(json->>'$.triggeredAt', '%Y-%m-%dT%T.%fZ'))
+ * </code> column in the 20210616-create-dismissible-notifications.yml liquibase change set.
+ */
 public class PrecisionSqlSerializer extends InstantSerializerBase<Instant> {
   public PrecisionSqlSerializer() {
     super(