Fix/receipts health (#1616)

* release lock on update progress (#1614)

* chore: remove the receipt

* chore: Remove the receipt

Co-authored-by: Aiden McClelland <3732071+dr-bonez@users.noreply.github.com>

backend/src/manager/health.rs

@@ -1,16 +1,95 @@
 use std::collections::BTreeMap;
 use std::sync::atomic::{AtomicBool, Ordering};
 
-use patch_db::{DbHandle, LockType};
+use patch_db::{DbHandle, LockReceipt, LockType};
 use tracing::instrument;
 
 use crate::context::RpcContext;
+use crate::db::model::CurrentDependents;
 use crate::dependencies::{break_transitive, heal_transitive, DependencyError};
-use crate::s9pk::manifest::PackageId;
+use crate::s9pk::manifest::{Manifest, PackageId};
 use crate::status::health_check::{HealthCheckId, HealthCheckResult};
 use crate::status::MainStatus;
 use crate::Error;
 
+struct HealthCheckPreInformationReceipt {
+    status_model: LockReceipt<MainStatus, ()>,
+    manifest: LockReceipt<Manifest, ()>,
+}
+impl HealthCheckPreInformationReceipt {
+    pub async fn new(db: &'_ mut impl DbHandle, id: &PackageId) -> Result<Self, Error> {
+        let mut locks = Vec::new();
+
+        let setup = Self::setup(&mut locks, id);
+        setup(&db.lock_all(locks).await?)
+    }
+
+    pub fn setup(
+        locks: &mut Vec<patch_db::LockTargetId>,
+        id: &PackageId,
+    ) -> impl FnOnce(&patch_db::Verifier) -> Result<Self, Error> {
+        let status_model = crate::db::DatabaseModel::new()
+            .package_data()
+            .idx_model(id)
+            .and_then(|x| x.installed())
+            .map(|x| x.status().main())
+            .make_locker(LockType::Read)
+            .add_to_keys(locks);
+        let manifest = crate::db::DatabaseModel::new()
+            .package_data()
+            .idx_model(id)
+            .and_then(|x| x.installed())
+            .map(|x| x.manifest())
+            .make_locker(LockType::Read)
+            .add_to_keys(locks);
+        move |skeleton_key| {
+            Ok(Self {
+                status_model: status_model.verify(skeleton_key)?,
+                manifest: manifest.verify(skeleton_key)?,
+            })
+        }
+    }
+}
+
+struct HealthCheckStatusReceipt {
+    status: LockReceipt<MainStatus, ()>,
+    current_dependents: LockReceipt<CurrentDependents, ()>,
+}
+impl HealthCheckStatusReceipt {
+    pub async fn new(db: &'_ mut impl DbHandle, id: &PackageId) -> Result<Self, Error> {
+        let mut locks = Vec::new();
+
+        let setup = Self::setup(&mut locks, id);
+        setup(&db.lock_all(locks).await?)
+    }
+
+    pub fn setup(
+        locks: &mut Vec<patch_db::LockTargetId>,
+        id: &PackageId,
+    ) -> impl FnOnce(&patch_db::Verifier) -> Result<Self, Error> {
+        let status = crate::db::DatabaseModel::new()
+            .package_data()
+            .idx_model(id)
+            .and_then(|x| x.installed())
+            .map(|x| x.status().main())
+            .make_locker(LockType::Write)
+            .add_to_keys(locks);
+        let current_dependents = crate::db::DatabaseModel::new()
+            .package_data()
+            .idx_model(id)
+            .and_then(|x| x.installed())
+            .map(|x| x.current_dependents())
+            .make_locker(LockType::Read)
+            .add_to_keys(locks);
+        move |skeleton_key| {
+            Ok(Self {
+                status: status.verify(skeleton_key)?,
+                current_dependents: current_dependents.verify(skeleton_key)?,
+            })
+        }
+    }
+}
+
 #[instrument(skip(ctx, db))]
 pub async fn check<Db: DbHandle>(
     ctx: &RpcContext,
@@ -19,35 +98,17 @@ pub async fn check<Db: DbHandle>(
     should_commit: &AtomicBool,
 ) -> Result<(), Error> {
     let mut tx = db.begin().await?;
+    let (manifest, started) = {
+        let mut checkpoint = tx.begin().await?;
+        let receipts = HealthCheckPreInformationReceipt::new(&mut checkpoint, id).await?;
 
-    let mut checkpoint = tx.begin().await?;
+        let manifest = receipts.manifest.get(&mut checkpoint).await?;
 
-    let installed_model = crate::db::DatabaseModel::new()
+        let started = receipts.status_model.get(&mut checkpoint).await?.started();
-        .package_data()
-        .idx_model(id)
-        .expect(&mut checkpoint)
-        .await?
-        .installed()
-        .expect(&mut checkpoint)
-        .await?;
 
-    let manifest = installed_model
+        checkpoint.save().await?;
-        .clone()
+        (manifest, started)
-        .manifest()
+    };
-        .get(&mut checkpoint, true)
-        .await?
-        .into_owned();
-
-    let started = installed_model
-        .clone()
-        .status()
-        .main()
-        .started()
-        .get(&mut checkpoint, true)
-        .await?
-        .into_owned();
-
-    checkpoint.save().await?;
 
     let health_results = if let Some(started) = started {
         manifest
@@ -61,48 +122,38 @@ pub async fn check<Db: DbHandle>(
     if !should_commit.load(Ordering::SeqCst) {
         return Ok(());
     }
+    let current_dependents = {
+        let mut checkpoint = tx.begin().await?;
+        let receipts = HealthCheckStatusReceipt::new(&mut checkpoint, id).await?;
 
-    let mut checkpoint = tx.begin().await?;
+        let status = receipts.status.get(&mut checkpoint).await?;
 
-    crate::db::DatabaseModel::new()
+        match status {
-        .package_data()
+            MainStatus::Running { health, started } => {
-        .lock(&mut checkpoint, LockType::Write)
+                receipts
-        .await?;
+                    .status
-
+                    .set(
-    let mut status = crate::db::DatabaseModel::new()
+                        &mut checkpoint,
-        .package_data()
+                        MainStatus::Running {
-        .idx_model(id)
+                            health: health_results.clone(),
-        .expect(&mut checkpoint)
+                            started,
-        .await?
+                        },
-        .installed()
+                    )
-        .expect(&mut checkpoint)
+                    .await?;
-        .await?
+            }
-        .status()
+            _ => (),
-        .main()
-        .get_mut(&mut checkpoint)
-        .await?;
-
-    match &mut *status {
-        MainStatus::Running { health, .. } => {
-            *health = health_results.clone();
         }
-        _ => (),
+        let current_dependents = receipts.current_dependents.get(&mut checkpoint).await?;
-    }
 
-    status.save(&mut checkpoint).await?;
+        checkpoint.save().await?;
-
+        current_dependents
-    let current_dependents = installed_model
+    };
-        .current_dependents()
-        .get(&mut checkpoint, true)
-        .await?;
-
-    checkpoint.save().await?;
 
     tracing::debug!("Checking health of {}", id);
     let receipts = crate::dependencies::BreakTransitiveReceipts::new(&mut tx).await?;
     tracing::debug!("Got receipts {}", id);
 
-    for (dependent, info) in (*current_dependents).0.iter() {
+    for (dependent, info) in (current_dependents).0.iter() {
         let failures: BTreeMap<HealthCheckId, HealthCheckResult> = health_results
             .iter()
             .filter(|(_, hc_res)| !matches!(hc_res, HealthCheckResult::Success { .. }))

backend/src/status/mod.rs

@@ -63,6 +63,16 @@ impl MainStatus {
             MainStatus::Stopped | MainStatus::Stopping | MainStatus::Restarting => (),
         }
     }
+    pub fn started(&self) -> Option<DateTime<Utc>> {
+        match self {
+            MainStatus::Running { started, .. } => Some(*started),
+            MainStatus::BackingUp { started, .. } => *started,
+            MainStatus::Stopped => None,
+            MainStatus::Restarting => None,
+            MainStatus::Stopping => None,
+            MainStatus::Starting { .. } => None,
+        }
+    }
 }
 impl MainStatusModel {
     pub fn started(self) -> Model<Option<DateTime<Utc>>> {