feat(core): re-run a published version's migration when its revision is bumped

projects/start-os/web/ui/src/app/services/api/mock-patch.ts

@@ -255,6 +255,7 @@ export const mockPatchData: DataModel = {
     unreadNotificationCount: 5,
     packageVersionCompat: '>=0.3.0 <=0.3.6',
     postInitMigrationTodos: {},
+    latestMigrationRevision: 0,
     statusInfo: {
       // currentBackup: null,
       updateProgress: null,

shared-libs/crates/start-core/VERSION_BUMP.md

@@ -132,3 +132,9 @@ The `up()` and `down()` methods handle database migrations:
 - **`down()`** — rolls back
 
 If no migration is needed, return `Ok(Value::Null)` from `up()` and `Ok(())` from `down()`. For complex migrations, set `type PreUpRes` to pass data from `pre_up()` into `up()`.
+
+### Changing a migration that has already been published
+
+Every master push publishes `Current` to alpha, so a server can be sitting on a version whose `up()` or `post_up()` has since changed — and `pre_init` only migrates a db whose `version` is _behind_ `Current`. A change to either after that version has reached any channel therefore needs its **`migration_revision()`** bumped (it defaults to `0`). `commit` records the revision it applied in `serverInfo.latestMigrationRevision`, and a server already on that version whose stored revision differs re-runs `up()` and `commit()` on its next boot; `commit` re-queues the version in `postInitMigrationTodos`, so `post_up()` runs again too.
+
+**Bumping the revision requires the migration to be idempotent with the previous revision.** The re-run happens on a db the earlier `up()` already transformed, never on the pre-migration shape, so the new `up()` must produce the same result whether it follows the previous revision or starts from the version before — and the same holds for `post_up()`. A migration that cannot satisfy that gets a new version node instead, which starts back at revision `0`.

shared-libs/crates/start-core/src/db/model/public.rs

@@ -61,6 +61,7 @@ impl Public {
                 last_backup: None,
                 package_version_compat: Current::default().compat().clone(),
                 post_init_migration_todos: BTreeMap::new(),
+                latest_migration_revision: Current::default().migration_revision(),
                 network: NetworkInfo {
                     host: Host {
                         bindings: Bindings(
@@ -180,6 +181,8 @@ pub struct ServerInfo {
     pub package_version_compat: VersionRange,
     #[ts(type = "Record<string, unknown>")]
     pub post_init_migration_todos: BTreeMap<Version, Value>,
+    #[serde(default)]
+    pub latest_migration_revision: usize,
     #[ts(type = "string | null")]
     pub last_backup: Option<DateTime<Utc>>,
     pub network: NetworkInfo,

shared-libs/crates/start-core/src/version/mod.rs

@@ -83,8 +83,9 @@ pub type Current = v0_4_0_2::Version; // VERSION_BUMP
 impl Current {
     #[instrument(skip(self, db))]
     pub async fn pre_init(self, db: &PatchDb) -> Result<(), Error> {
+        let mut dump = db.dump(&ROOT).await;
         let from = from_value::<Version>(
-            version_accessor(&mut db.dump(&ROOT).await.value)
+            version_accessor(&mut dump.value)
                 .or_not_found("`version` in db")?
                 .clone(),
         )?
@@ -108,11 +109,22 @@ impl Current {
                 .result?;
             }
             Ordering::Equal => {
-                db.apply_function(|db| {
-                    Ok::<_, Error>((to_value(&from_value::<Database>(db.clone())?)?, ()))
-                })
-                .await
-                .result?;
+                if applied_migration_revision(&mut dump.value)? == self.migration_revision() {
+                    db.apply_function(|db| {
+                        Ok::<_, Error>((to_value(&from_value::<Database>(db.clone())?)?, ()))
+                    })
+                    .await
+                    .result?;
+                } else {
+                    let pre_up = self.pre_up().await?;
+                    db.apply_function(|mut db| {
+                        let res = self.up(&mut db, pre_up)?;
+                        self.commit(&mut db, res)?;
+                        Ok::<_, Error>((to_value(&from_value::<Database>(db.clone())?)?, ()))
+                    })
+                    .await
+                    .result?;
+                }
             }
         }
         Ok(())
@@ -431,6 +443,28 @@ fn post_init_migration_todos_accessor(db: &mut Value) -> Option<&mut Value> {
     server_info.get_mut("postInitMigrationTodos")
 }
 
+#[instrument(skip_all)]
+fn latest_migration_revision_accessor(db: &mut Value) -> Option<&mut Value> {
+    let server_info = if db.get("public").is_some() {
+        db.get_mut("public")?.get_mut("serverInfo")?
+    } else {
+        db.get_mut("server-info")?
+    };
+    if server_info.get("latestMigrationRevision").is_none() {
+        server_info
+            .as_object_mut()?
+            .insert("latestMigrationRevision".into(), Value::from(0usize));
+    }
+    server_info.get_mut("latestMigrationRevision")
+}
+
+fn applied_migration_revision(db: &mut Value) -> Result<usize, Error> {
+    latest_migration_revision_accessor(db)
+        .map(|v| from_value::<usize>(v.clone()))
+        .transpose()
+        .map(Option::unwrap_or_default)
+}
+
 struct PreUps {
     prev: Option<Box<PreUps>>,
     value: Box<dyn Any + UnwindSafe + Send + 'static>,
@@ -523,6 +557,12 @@ where
     type PreUpRes: Send + UnwindSafe;
     fn semver(self) -> exver::Version;
     fn compat(self) -> &'static exver::VersionRange;
+    /// Bump when `up` or `post_up` changes after this version has been published to a channel.
+    /// Both then re-run over a db the previous revision already migrated, so they MUST be
+    /// idempotent with it.
+    fn migration_revision(self) -> usize {
+        0
+    }
     /// MUST be idempotent, and is run before *all* db migrations
     fn pre_up(self) -> impl Future<Output = Result<Self::PreUpRes, Error>> + Send + 'static;
     fn up(self, db: &mut Value, input: Self::PreUpRes) -> Result<Value, Error> {
@@ -564,6 +604,9 @@ where
                 ErrorKind::Database,
             ));
         }
+        *latest_migration_revision_accessor(db)
+            .or_not_found("`public.serverInfo.latestMigrationRevision` in db")? =
+            to_value(&self.migration_revision())?;
         Ok(())
     }
 }
@@ -716,6 +759,7 @@ pub fn git_info() -> Result<InternedString, Error> {
 
 #[cfg(test)]
 mod tests {
+    use imbl_value::json;
     use proptest::prelude::*;
 
     use super::*;
@@ -731,6 +775,36 @@ mod tests {
         );
     }
 
+    #[test]
+    fn a_db_that_predates_revisions_reads_as_revision_zero() {
+        let mut db = json!({ "public": { "serverInfo": {} } });
+        assert_eq!(applied_migration_revision(&mut db).unwrap(), 0);
+        assert_eq!(
+            db["public"]["serverInfo"]["latestMigrationRevision"],
+            json!(0)
+        );
+    }
+
+    #[test]
+    fn commit_records_the_revision_of_the_migration_it_applied() {
+        let mut db = json!({ "public": { "serverInfo": {
+            "version": "0.4.0.1",
+            "packageVersionCompat": ">=0.3.0 <0.5.0",
+            "postInitMigrationTodos": {},
+            "latestMigrationRevision": 7,
+        } } });
+        let current = Current::default();
+        current.commit(&mut db, Value::Null).unwrap();
+        assert_eq!(
+            applied_migration_revision(&mut db).unwrap(),
+            current.migration_revision()
+        );
+        assert_eq!(
+            db["public"]["serverInfo"]["version"],
+            json!(current.semver().to_string())
+        );
+    }
+
     fn em_version() -> impl Strategy<Value = exver::Version> {
         any::<(usize, usize, usize, bool)>().prop_map(|(major, minor, patch, alpha)| {
             if alpha {

shared-libs/crates/start-core/src/version/v0_4_0_2.rs

@@ -27,6 +27,9 @@ impl VersionT for Version {
     fn compat(self) -> &'static VersionRange {
         &V0_3_0_COMPAT
     }
+    fn migration_revision(self) -> usize {
+        1
+    }
     #[instrument(skip_all)]
     fn up(self, db: &mut Value, _: Self::PreUpRes) -> Result<Value, Error> {
         rehome_admin_ui_port(db);

shared-libs/ts-modules/start-core/lib/osBindings/ServerInfo.ts

@@ -12,6 +12,7 @@ export type ServerInfo = {
   version: string
   packageVersionCompat: string
   postInitMigrationTodos: Record<string, unknown>
+  latestMigrationRevision: number
   lastBackup: string | null
   network: NetworkInfo
   statusInfo: ServerStatus