Expand Visit Town database migration (#8644)

docs/SQLITE_ACTION_RESULT_DECOMPOSITION.md

@@ -156,6 +156,7 @@ CREATE TABLE action_changed_provinces (
     support_delta                         REAL,
     -- round properties
     set_has_acted                         INTEGER,  -- 0/1/NULL
+    set_ruler_is_traveling                INTEGER,  -- legacy, dual-written during Visit Town migration
     set_ruler_is_visiting_town            INTEGER,
     -- ruling faction changes
     clear_ruling_faction_id               INTEGER NOT NULL DEFAULT 0,

src/main/scala/net/eagle0/eagle/service/ChangedProvinceTableWriter.scala

@@ -60,7 +60,7 @@ private[service] object ChangedProvinceTableWriter {
       |    infrastructure_devastation_delta    REAL,
       |    support_delta                       REAL,
       |    set_has_acted                       INTEGER,
-      |    set_ruler_is_visiting_town          INTEGER,
+      |    set_ruler_is_traveling              INTEGER,
       |    clear_ruling_faction_id             INTEGER NOT NULL DEFAULT 0,
       |    new_ruling_faction_id               INTEGER,
       |    new_locked_improvement_kind         TEXT,
@@ -79,6 +79,30 @@ private[service] object ChangedProvinceTableWriter {
   val renameVisitTownColumnStatement: String =
     "ALTER TABLE action_changed_provinces RENAME COLUMN set_ruler_is_traveling TO set_ruler_is_visiting_town"
 
+  private val addVisitTownColumnStatement: String         =
+    "ALTER TABLE action_changed_provinces ADD COLUMN set_ruler_is_visiting_town INTEGER"
+  private val addLegacyTravelColumnStatement: String      =
+    "ALTER TABLE action_changed_provinces ADD COLUMN set_ruler_is_traveling INTEGER"
+  private val backfillVisitTownColumnStatement: String    =
+    "UPDATE action_changed_provinces SET set_ruler_is_visiting_town = set_ruler_is_traveling " +
+      "WHERE set_ruler_is_visiting_town IS NULL"
+  private val backfillLegacyTravelColumnStatement: String =
+    "UPDATE action_changed_provinces SET set_ruler_is_traveling = set_ruler_is_visiting_town " +
+      "WHERE set_ruler_is_traveling IS NULL"
+
+  private[service] def visitTownExpansionStatements(
+      hasLegacyTravelColumn: Boolean,
+      hasVisitTownColumn: Boolean
+  ): Vector[String] = {
+    // The expand release keeps both columns and dual-writes them. A later contract release can backfill once more,
+    // switch readers/writers to the Visit Town column only, and finally drop the legacy column.
+    val addStatements =
+      Option.when(!hasLegacyTravelColumn)(addLegacyTravelColumnStatement).toVector ++
+        Option.when(!hasVisitTownColumn)(addVisitTownColumnStatement)
+    if addStatements.isEmpty then Vector.empty
+    else addStatements ++ Vector(backfillVisitTownColumnStatement, backfillLegacyTravelColumnStatement)
+  }
+
   /**
    * Write one `ChangedProvince` for `(actionSeq, n, cp.id)`: the v2 scalar columns (denormalized for querying) plus the
    * full proto in `changed_province_proto`. `n` is the position within the source `ActionResult`'s `changedProvinces`
@@ -105,11 +129,12 @@ private[service] object ChangedProvinceTableWriter {
       """INSERT INTO action_changed_provinces
         |(action_seq, n, province_id, gold_delta, food_delta, new_price_index, economy_delta, agriculture_delta,
         | infrastructure_delta, economy_devastation_delta, agriculture_devastation_delta,
-        | infrastructure_devastation_delta, support_delta, set_has_acted, set_ruler_is_visiting_town,
+        | infrastructure_devastation_delta, support_delta, set_has_acted, set_ruler_is_traveling,
+        | set_ruler_is_visiting_town,
         | clear_ruling_faction_id, new_ruling_faction_id, new_locked_improvement_kind, new_locked_improvement_value,
         | new_province_orders, clear_defending_army, cleared_pending_conquest_info, removed_deferred_change_index,
         | changed_province_proto)
-        |VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)""".stripMargin
+        |VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)""".stripMargin
     )
     try {
       ps.setInt(1, actionSeq)
@@ -127,15 +152,16 @@ private[service] object ChangedProvinceTableWriter {
       setDoubleOpt(ps, 13, cp.supportDelta)
       setBoolOpt(ps, 14, cp.setHasActed)
       setBoolOpt(ps, 15, cp.setRulerIsVisitingTown)
-      ps.setInt(16, if cp.clearRulingFactionId then 1 else 0)
-      setIntOpt(ps, 17, cp.newRulingFactionId)
-      setStrOpt(ps, 18, lockedKind)
-      setIntOpt(ps, 19, lockedValue)
-      setIntOpt(ps, 20, provinceOrders)
-      ps.setInt(21, if cp.clearDefendingArmy then 1 else 0)
-      ps.setInt(22, if cp.clearPendingConquestInfo then 1 else 0)
-      setIntOpt(ps, 23, cp.removedDeferredChangeIndex)
-      ps.setBytes(24, cp.toByteArray)
+      setBoolOpt(ps, 16, cp.setRulerIsVisitingTown)
+      ps.setInt(17, if cp.clearRulingFactionId then 1 else 0)
+      setIntOpt(ps, 18, cp.newRulingFactionId)
+      setStrOpt(ps, 19, lockedKind)
+      setIntOpt(ps, 20, lockedValue)
+      setIntOpt(ps, 21, provinceOrders)
+      ps.setInt(22, if cp.clearDefendingArmy then 1 else 0)
+      ps.setInt(23, if cp.clearPendingConquestInfo then 1 else 0)
+      setIntOpt(ps, 24, cp.removedDeferredChangeIndex)
+      ps.setBytes(25, cp.toByteArray)
       ps.executeUpdate(): Unit
     } finally ps.close()
     end try

src/main/scala/net/eagle0/eagle/service/PostgresGameSchemaPool.scala

@@ -41,7 +41,8 @@ private[service] object PostgresGameSchemaPool {
   private val PoolLockName                  = "eagle0_game_schema_pool"
   private val PostCreationRefillDelayMillis = 1000L
   private val PoolPrefix                    =
-    s"eagle_pool_h${PostgresHistory.SchemaVersion}_t${PostgresClientTextStore.SchemaVersion}_"
+    s"eagle_pool_h${PostgresHistory.SchemaVersion}_l${PostgresHistory.SchemaLayoutVersion}_" +
+      s"t${PostgresClientTextStore.SchemaVersion}_"
 
   private val started                = new AtomicBoolean(false)
   private val delayedRefillScheduled = new AtomicBoolean(false)

src/main/scala/net/eagle0/eagle/service/PostgresHistory.scala

@@ -682,9 +682,12 @@ class PostgresHistory private[service] (
 }
 
 object PostgresHistory {
-  private val SnapshotInterval       = 25
-  private[service] val SchemaVersion = 1
-  private val applier                = SqliteHistory.applier
+  private val SnapshotInterval             = 25
+  // Keep v1 during the additive phase so the old blue/green container can still lazy-load expanded schemas.
+  private[service] val SchemaVersion       = 1
+  // Prepared schemas need a new namespace even though the additive layout remains readable by schema-v1 servers.
+  private[service] val SchemaLayoutVersion = 2
+  private val applier                      = SqliteHistory.applier
 
   val ActionResultInsertSql: String =
     """INSERT INTO action_results
@@ -884,7 +887,7 @@ object PostgresHistory {
   private[service] def initializeSchema(connection: Connection): Unit = {
     execEach(connection, Vector("CREATE TABLE IF NOT EXISTS metadata (key TEXT PRIMARY KEY, value TEXT NOT NULL)"))
     val version = readSchemaVersion(connection)
-    if version == SchemaVersion then ()
+    if version == SchemaVersion then ensureVisitTownExpansion(connection)
     else if version == 0 then {
       val originalAutoCommit = connection.getAutoCommit
       connection.setAutoCommit(false)
@@ -904,6 +907,37 @@ object PostgresHistory {
     end if
   }
 
+  private def ensureVisitTownExpansion(connection: Connection): Unit = {
+    val statements = ChangedProvinceTableWriter.visitTownExpansionStatements(
+      hasLegacyTravelColumn = columnExists(connection, "action_changed_provinces", "set_ruler_is_traveling"),
+      hasVisitTownColumn = columnExists(connection, "action_changed_provinces", "set_ruler_is_visiting_town")
+    )
+    if statements.nonEmpty then {
+      val originalAutoCommit = connection.getAutoCommit
+      connection.setAutoCommit(false)
+      try {
+        execEach(connection, statements)
+        connection.commit()
+      } catch {
+        case t: Throwable =>
+          connection.rollback()
+          throw t
+      } finally connection.setAutoCommit(originalAutoCommit)
+    }
+  }
+
+  private def columnExists(connection: Connection, tableName: String, columnName: String): Boolean = {
+    val stmt = connection.prepareStatement(
+      """SELECT 1 FROM information_schema.columns
+        |WHERE table_schema = current_schema() AND table_name = ? AND column_name = ?""".stripMargin
+    )
+    try {
+      stmt.setString(1, tableName)
+      stmt.setString(2, columnName)
+      stmt.executeQuery().next()
+    } finally stmt.close()
+  }
+
   private def readSchemaVersion(connection: Connection): Int = {
     val stmt = connection.prepareStatement("SELECT value FROM metadata WHERE key = 'schema_version'")
     try {
@@ -1144,6 +1178,10 @@ object PostgresHistory {
         |)""".stripMargin
     ) ++
       ChangedProvinceTableWriter.schemaStatements ++
+      ChangedProvinceTableWriter.visitTownExpansionStatements(
+        hasLegacyTravelColumn = true,
+        hasVisitTownColumn = false
+      ) ++
       ChangedHeroTableWriter.schemaStatements ++
       ChangedFactionTableWriter.schemaStatements ++
       ChangedBattalionTableWriter.schemaStatements ++

src/main/scala/net/eagle0/eagle/service/SqliteHistory.scala

@@ -1742,7 +1742,8 @@ object SqliteHistory {
       Migration(21, applyV21DropActionResultsPayload),
       Migration(22, applyV22ShardokAppliedDeltas),
       Migration(23, applyV23DecisionRecords),
-      Migration(24, applyV24RenameVisitTownColumn)
+      Migration(24, applyV24RenameVisitTownColumn),
+      Migration(25, applyV25ExpandVisitTownColumns)
     )
 
     val latestVersion: Int = all.lastOption.map(_.version).getOrElse(0)
@@ -2079,6 +2080,24 @@ object SqliteHistory {
       if tableExists(connection, "action_changed_provinces") &&
         columnExists(connection, "action_changed_provinces", "set_ruler_is_traveling")
       then execEach(connection, Vector(ChangedProvinceTableWriter.renameVisitTownColumnStatement))
+
+    private def applyV25ExpandVisitTownColumns(connection: Connection): Unit =
+      if tableExists(connection, "action_changed_provinces") then
+        execEach(
+          connection,
+          ChangedProvinceTableWriter.visitTownExpansionStatements(
+            hasLegacyTravelColumn = columnExists(
+              connection,
+              "action_changed_provinces",
+              "set_ruler_is_traveling"
+            ),
+            hasVisitTownColumn = columnExists(
+              connection,
+              "action_changed_provinces",
+              "set_ruler_is_visiting_town"
+            )
+          )
+        )
   }
 
   /**

src/test/scala/net/eagle0/eagle/service/BUILD.bazel

@@ -39,6 +39,7 @@ scala_test(
     srcs = ["PostgresHistorySchemaBatchTest.scala"],
     deps = [
         "//src/main/scala/net/eagle0/eagle:eagle_pkg",
+        "//src/main/scala/net/eagle0/eagle/service:changed_province_table_writer",
         "//src/main/scala/net/eagle0/eagle/service:sqlite_history",
         "@maven//:com_google_protobuf_protobuf_java",
         "@maven//:org_scalamock_scalamock_3",

src/test/scala/net/eagle0/eagle/service/PostgresGameSchemaPoolTest.scala

@@ -42,10 +42,10 @@ class PostgresGameSchemaPoolTest extends AnyFlatSpec with Matchers with MockFact
     events shouldBe Vector("refill")
   }
 
-  "poolSchemaNames" should "include both schema versions" in {
+  "poolSchemaNames" should "include history, layout, and client text schema versions" in {
     PostgresGameSchemaPool.poolSchemaNames(2) shouldBe Vector(
-      "eagle_pool_h1_t2_0",
-      "eagle_pool_h1_t2_1"
+      "eagle_pool_h1_l2_t2_0",
+      "eagle_pool_h1_l2_t2_1"
     )
   }
 
@@ -68,13 +68,13 @@ class PostgresGameSchemaPoolTest extends AnyFlatSpec with Matchers with MockFact
       (() => targetStatement.close()).expects().once(): Unit
 
       (connection.prepareStatement(_: String)).expects(schemaExistsSql).returning(firstStatement).once(): Unit
-      firstStatement.setString.expects(1, "eagle_pool_h1_t2_0").once(): Unit
+      firstStatement.setString.expects(1, "eagle_pool_h1_l2_t2_0").once(): Unit
       (() => firstStatement.executeQuery()).expects().returning(firstResult).once(): Unit
       (() => firstResult.next()).expects().returning(false).once(): Unit
       (() => firstStatement.close()).expects().once(): Unit
 
       (connection.prepareStatement(_: String)).expects(schemaExistsSql).returning(secondStatement).once(): Unit
-      secondStatement.setString.expects(1, "eagle_pool_h1_t2_1").once(): Unit
+      secondStatement.setString.expects(1, "eagle_pool_h1_l2_t2_1").once(): Unit
       (() => secondStatement.executeQuery()).expects().returning(secondResult).once(): Unit
       (() => secondResult.next()).expects().returning(true).once(): Unit
       (() => secondStatement.close()).expects().once(): Unit
@@ -82,7 +82,7 @@ class PostgresGameSchemaPoolTest extends AnyFlatSpec with Matchers with MockFact
       (() => connection.createStatement()).expects().returning(renameStatement).once(): Unit
       (renameStatement
         .execute(_: String))
-        .expects("ALTER SCHEMA eagle_pool_h1_t2_1 RENAME TO eagle_game_1234")
+        .expects("ALTER SCHEMA eagle_pool_h1_l2_t2_1 RENAME TO eagle_game_1234")
         .returning(false)
         .once(): Unit
       (() => renameStatement.close()).expects().once(): Unit

src/test/scala/net/eagle0/eagle/service/PostgresHistorySchemaBatchTest.scala

@@ -1,6 +1,6 @@
 package net.eagle0.eagle.service
 
-import java.sql.{Connection, ResultSet, Statement}
+import java.sql.{Connection, PreparedStatement, ResultSet, Statement}
 
 import com.google.protobuf.ByteString
 import org.scalamock.scalatest.MockFactory
@@ -8,6 +8,110 @@ import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
 
 class PostgresHistorySchemaBatchTest extends AnyFlatSpec with Matchers with MockFactory {
+  "Visit Town schema expansion" should "add and backfill the new column for legacy schemas" in {
+    ChangedProvinceTableWriter.visitTownExpansionStatements(
+      hasLegacyTravelColumn = true,
+      hasVisitTownColumn = false
+    ) shouldBe Vector(
+      "ALTER TABLE action_changed_provinces ADD COLUMN set_ruler_is_visiting_town INTEGER",
+      "UPDATE action_changed_provinces SET set_ruler_is_visiting_town = set_ruler_is_traveling " +
+        "WHERE set_ruler_is_visiting_town IS NULL",
+      "UPDATE action_changed_provinces SET set_ruler_is_traveling = set_ruler_is_visiting_town " +
+        "WHERE set_ruler_is_traveling IS NULL"
+    )
+  }
+
+  it should "restore and backfill the legacy column for schemas created by the rename release" in {
+    val statements = ChangedProvinceTableWriter.visitTownExpansionStatements(
+      hasLegacyTravelColumn = false,
+      hasVisitTownColumn = true
+    )
+    statements.head shouldBe "ALTER TABLE action_changed_provinces ADD COLUMN set_ruler_is_traveling INTEGER"
+    statements.last shouldBe
+      "UPDATE action_changed_provinces SET set_ruler_is_traveling = set_ruler_is_visiting_town " +
+      "WHERE set_ruler_is_traveling IS NULL"
+  }
+
+  it should "leave an already-expanded schema unchanged" in {
+    ChangedProvinceTableWriter.visitTownExpansionStatements(
+      hasLegacyTravelColumn = true,
+      hasVisitTownColumn = true
+    ) shouldBe empty
+  }
+
+  it should "expand an existing PostgreSQL v1 schema without changing its version" in {
+    val connection         = mock[Connection]
+    val metadataStatement  = mock[Statement]
+    val versionStatement   = mock[PreparedStatement]
+    val versionResult      = mock[ResultSet]
+    val legacyStatement    = mock[PreparedStatement]
+    val legacyResult       = mock[ResultSet]
+    val visitTownStatement = mock[PreparedStatement]
+    val visitTownResult    = mock[ResultSet]
+    val expansionStatement = mock[Statement]
+    val columnExistsSql    =
+      "SELECT 1 FROM information_schema.columns\n" +
+        "WHERE table_schema = current_schema() AND table_name = ? AND column_name = ?"
+
+    inSequence {
+      (() => connection.createStatement()).expects().returning(metadataStatement).once(): Unit
+      metadataStatement.addBatch
+        .expects("CREATE TABLE IF NOT EXISTS metadata (key TEXT PRIMARY KEY, value TEXT NOT NULL)")
+        .once(): Unit
+      (() => metadataStatement.executeBatch()).expects().returning(Array(0)).once(): Unit
+      (() => metadataStatement.close()).expects().once(): Unit
+
+      (connection
+        .prepareStatement(_: String))
+        .expects("SELECT value FROM metadata WHERE key = 'schema_version'")
+        .returning(versionStatement)
+        .once(): Unit
+      (() => versionStatement.executeQuery()).expects().returning(versionResult).once(): Unit
+      (() => versionResult.next()).expects().returning(true).once(): Unit
+      (versionResult.getString(_: Int)).expects(1).returning("1").once(): Unit
+      (() => versionStatement.close()).expects().once(): Unit
+
+      (connection.prepareStatement(_: String)).expects(columnExistsSql).returning(legacyStatement).once(): Unit
+      legacyStatement.setString.expects(1, "action_changed_provinces").once(): Unit
+      legacyStatement.setString.expects(2, "set_ruler_is_traveling").once(): Unit
+      (() => legacyStatement.executeQuery()).expects().returning(legacyResult).once(): Unit
+      (() => legacyResult.next()).expects().returning(true).once(): Unit
+      (() => legacyStatement.close()).expects().once(): Unit
+
+      (connection.prepareStatement(_: String)).expects(columnExistsSql).returning(visitTownStatement).once(): Unit
+      visitTownStatement.setString.expects(1, "action_changed_provinces").once(): Unit
+      visitTownStatement.setString.expects(2, "set_ruler_is_visiting_town").once(): Unit
+      (() => visitTownStatement.executeQuery()).expects().returning(visitTownResult).once(): Unit
+      (() => visitTownResult.next()).expects().returning(false).once(): Unit
+      (() => visitTownStatement.close()).expects().once(): Unit
+
+      (() => connection.getAutoCommit).expects().returning(true).once(): Unit
+      connection.setAutoCommit.expects(false).once(): Unit
+      (() => connection.createStatement()).expects().returning(expansionStatement).once(): Unit
+      expansionStatement.addBatch
+        .expects("ALTER TABLE action_changed_provinces ADD COLUMN set_ruler_is_visiting_town INTEGER")
+        .once(): Unit
+      expansionStatement.addBatch
+        .expects(
+          "UPDATE action_changed_provinces SET set_ruler_is_visiting_town = set_ruler_is_traveling " +
+            "WHERE set_ruler_is_visiting_town IS NULL"
+        )
+        .once(): Unit
+      expansionStatement.addBatch
+        .expects(
+          "UPDATE action_changed_provinces SET set_ruler_is_traveling = set_ruler_is_visiting_town " +
+            "WHERE set_ruler_is_traveling IS NULL"
+        )
+        .once(): Unit
+      (() => expansionStatement.executeBatch()).expects().returning(Array(0, 0, 0)).once(): Unit
+      (() => expansionStatement.close()).expects().once(): Unit
+      (() => connection.commit()).expects().once(): Unit
+      connection.setAutoCommit.expects(true).once(): Unit
+    }
+
+    PostgresHistory.initializeSchema(connection)
+  }
+
   "PostgresHistory.execEach" should "execute schema statements as one ordered JDBC batch" in {
     val connection = mock[Connection]
     val statement  = mock[Statement]

src/test/scala/net/eagle0/eagle/service/SqliteHistoryTest.scala

@@ -1072,6 +1072,34 @@ class SqliteHistoryTest extends AnyFlatSpec with Matchers with BeforeAndAfterEac
 
     withRawConnection { conn =>
       readSchemaVersion(conn) shouldBe Some(SqliteHistory.latestSchemaVersion)
+      columnsOf(conn, "action_changed_provinces") should contain allOf (
+        "set_ruler_is_traveling",
+        "set_ruler_is_visiting_town"
+      )
+    }
+  }
+
+  it should "expand a v24 Visit Town schema for dual writes" in {
+    val history = SqliteHistory.create(dbFile, emptyStartingState)
+    history.close()
+
+    withRawConnection { conn =>
+      val stmt = conn.createStatement()
+      try {
+        val _ = stmt.execute("ALTER TABLE action_changed_provinces DROP COLUMN set_ruler_is_traveling")
+        val _ = stmt.execute("UPDATE metadata SET value = '24' WHERE key = 'schema_version'")
+      } finally stmt.close()
+    }
+
+    val reloaded = SqliteHistory.loaded(dbFile)
+    reloaded.close()
+
+    withRawConnection { conn =>
+      readSchemaVersion(conn) shouldBe Some(SqliteHistory.latestSchemaVersion)
+      columnsOf(conn, "action_changed_provinces") should contain allOf (
+        "set_ruler_is_traveling",
+        "set_ruler_is_visiting_town"
+      )
     }
   }
 
@@ -2307,7 +2335,7 @@ class SqliteHistoryTest extends AnyFlatSpec with Matchers with BeforeAndAfterEac
         changedBattalions = Vector(ChangedBattalionC(battalion(id = 1, name = "Vanguard"))),
         changedFactions = Vector(ChangedFactionC(factionId = 3)),
         changedHeroes = Vector(ChangedHeroC(heroId = 7)),
-        changedProvinces = Vector(ChangedProvinceC(provinceId = 11)),
+        changedProvinces = Vector(ChangedProvinceC(provinceId = 11, setRulerIsVisitingTown = Some(true))),
         newBattalions = Vector(battalion(id = 21, name = "Reserve")),
         newFactions = Vector(faction(id = 31, name = "Crown")),
         newHeroes = Vector(hero(id = 41, nameTextId = "name-41")),
@@ -2348,14 +2376,23 @@ class SqliteHistoryTest extends AnyFlatSpec with Matchers with BeforeAndAfterEac
       withRawConnection { conn =>
         val stmt = conn.createStatement()
         try {
-          val emptyTables = perEntityTables.filter { table =>
+          val emptyTables      = perEntityTables.filter { table =>
             val rs = stmt.executeQuery(s"SELECT COUNT(*) FROM $table")
             rs.next() && rs.getInt(1) == 0
           }
           withClue(s"these per-entity tables were not populated by withNewResults: ${emptyTables.mkString(", ")}") {
             emptyTables shouldBe Vector.empty[String]
           }
+          val visitTownColumns = stmt.executeQuery(
+            "SELECT set_ruler_is_traveling, set_ruler_is_visiting_town FROM action_changed_provinces"
+          )
+          try {
+            visitTownColumns.next() shouldBe true
+            visitTownColumns.getInt("set_ruler_is_traveling") shouldBe 1
+            visitTownColumns.getInt("set_ruler_is_visiting_town") shouldBe 1
+          } finally visitTownColumns.close()
         } finally stmt.close()
+        end try
       }
     }