From c304777a3574bb912dac8bfff78561830d99c68b Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Wed, 27 Dec 2023 23:12:23 -0800
Subject: [PATCH] op heads: remove promote_new_op()

`OpHeadsStoreLock::promote_new_op()` doesn't add much over the new
`update_op_heads()`, so let's switch to the latter.
---
 lib/src/op_heads_store.rs        |  8 +++-----
 lib/src/simple_op_heads_store.rs | 14 ++------------
 lib/src/transaction.rs           | 10 ++++++----
 3 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/lib/src/op_heads_store.rs b/lib/src/op_heads_store.rs
index b478f1f2f..c92895395 100644
--- a/lib/src/op_heads_store.rs
+++ b/lib/src/op_heads_store.rs
@@ -35,9 +35,7 @@ pub enum OpHeadResolutionError<E> {
     Err(#[source] E),
 }
 
-pub trait OpHeadsStoreLock<'a> {
-    fn promote_new_op(&self, new_op: &Operation);
-}
+pub trait OpHeadsStoreLock<'a> {}
 
 /// Manages the set of current heads of the operation log.
 pub trait OpHeadsStore: Send + Sync + Debug {
@@ -109,7 +107,7 @@ pub fn resolve_op_heads<E>(
     // Note that the locking isn't necessary for correctness; we take the lock
     // only to prevent other concurrent processes from doing the same work (and
     // producing another set of divergent heads).
-    let lock = op_heads_store.lock();
+    let _lock = op_heads_store.lock();
     let op_head_ids = op_heads_store.get_op_heads();
 
     if op_head_ids.is_empty() {
@@ -139,7 +137,7 @@ pub fn resolve_op_heads<E>(
     op_heads.sort_by_key(|op| op.store_operation().metadata.end_time.timestamp.clone());
     match resolver(op_heads) {
         Ok(new_op) => {
-            lock.promote_new_op(&new_op);
+            op_heads_store.update_op_heads(new_op.parent_ids(), new_op.id());
             Ok(new_op)
         }
         Err(e) => Err(OpHeadResolutionError::Err(e)),
diff --git a/lib/src/simple_op_heads_store.rs b/lib/src/simple_op_heads_store.rs
index 24af51025..e20172a4f 100644
--- a/lib/src/simple_op_heads_store.rs
+++ b/lib/src/simple_op_heads_store.rs
@@ -22,7 +22,6 @@ use crate::backend::ObjectId;
 use crate::lock::FileLock;
 use crate::op_heads_store::{OpHeadsStore, OpHeadsStoreLock};
 use crate::op_store::OperationId;
-use crate::operation::Operation;
 
 pub struct SimpleOpHeadsStore {
     dir: PathBuf,
@@ -53,19 +52,11 @@ impl SimpleOpHeadsStore {
     }
 }
 
-struct SimpleOpHeadsStoreLock<'a> {
-    store: &'a dyn OpHeadsStore,
+struct SimpleOpHeadsStoreLock {
     _lock: FileLock,
 }
 
-impl OpHeadsStoreLock<'_> for SimpleOpHeadsStoreLock<'_> {
-    fn promote_new_op(&self, new_op: &Operation) {
-        self.store.add_op_head(new_op.id());
-        for old_id in new_op.parent_ids() {
-            self.store.remove_op_head(old_id);
-        }
-    }
-}
+impl OpHeadsStoreLock<'_> for SimpleOpHeadsStoreLock {}
 
 impl OpHeadsStore for SimpleOpHeadsStore {
     fn name(&self) -> &str {
@@ -105,7 +96,6 @@ impl OpHeadsStore for SimpleOpHeadsStore {
 
     fn lock<'a>(&'a self) -> Box<dyn OpHeadsStoreLock<'a> + 'a> {
         Box::new(SimpleOpHeadsStoreLock {
-            store: self,
             _lock: FileLock::lock(self.dir.join("lock")),
         })
     }
diff --git a/lib/src/transaction.rs b/lib/src/transaction.rs
index eeef3f208..11c1b34c7 100644
--- a/lib/src/transaction.rs
+++ b/lib/src/transaction.rs
@@ -175,10 +175,12 @@ impl UnpublishedOperation {
 
     pub fn publish(mut self) -> Arc<ReadonlyRepo> {
         let data = self.data.take().unwrap();
-        self.repo_loader
-            .op_heads_store()
-            .lock()
-            .promote_new_op(&data.operation);
+        {
+            let _lock = self.repo_loader.op_heads_store().lock();
+            self.repo_loader
+                .op_heads_store()
+                .update_op_heads(data.operation.parent_ids(), data.operation.id());
+        }
         let repo = self
             .repo_loader
             .create_from(data.operation, data.view, data.index);