Clean up the UnpublishedOperation struct

lib/src/transaction.rs

@@ -20,6 +20,7 @@ use itertools::Itertools as _;
 
 use crate::backend::Timestamp;
 use crate::index::ReadonlyIndex;
+use crate::op_heads_store::OpHeadsStore;
 use crate::op_store::OperationMetadata;
 use crate::operation::Operation;
 use crate::repo::{MutableRepo, ReadonlyRepo, Repo, RepoLoader, RepoLoaderError};
@@ -121,7 +122,7 @@ impl Transaction {
             .index_store()
             .write_index(mut_index, operation.id())
             .unwrap();
-        UnpublishedOperation::new(base_repo.loader(), operation, view, index)
+        UnpublishedOperation::new(&base_repo.loader(), operation, view, index)
     }
 }
 
@@ -147,70 +148,43 @@ pub fn create_op_metadata(
     }
 }
 
-struct NewRepoData {
-    operation: Operation,
-    view: View,
-    index: Box<dyn ReadonlyIndex>,
-}
-
+/// An Operation which has been written to the operation store but not
+/// published. The repo can be loaded at an unpublished Operation, but the
+/// Operation will not be visible in the op log if the repo is loaded at head.
+///
+/// Either [`Self::publish`] or [`Self::leave_unpublished`] must be called to
+/// finish the operation.
+#[must_use = "Either publish() or leave_unpublished() must be called to finish the operation."]
 pub struct UnpublishedOperation {
-    repo_loader: RepoLoader,
-    data: Option<NewRepoData>,
-    closed: bool,
+    op_heads_store: Arc<dyn OpHeadsStore>,
+    repo: Arc<ReadonlyRepo>,
 }
 
 impl UnpublishedOperation {
     fn new(
-        repo_loader: RepoLoader,
+        repo_loader: &RepoLoader,
         operation: Operation,
         view: View,
         index: Box<dyn ReadonlyIndex>,
     ) -> Self {
-        let data = Some(NewRepoData {
-            operation,
-            view,
-            index,
-        });
         UnpublishedOperation {
-            repo_loader,
-            data,
-            closed: false,
+            op_heads_store: repo_loader.op_heads_store().clone(),
+            repo: repo_loader.create_from(operation, view, index),
         }
     }
 
     pub fn operation(&self) -> &Operation {
-        &self.data.as_ref().unwrap().operation
+        self.repo.operation()
     }
 
-    pub fn publish(mut self) -> Arc<ReadonlyRepo> {
-        let data = self.data.take().unwrap();
-        {
-            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);
-        self.closed = true;
-        repo
+    pub fn publish(self) -> Arc<ReadonlyRepo> {
+        let _lock = self.op_heads_store.lock();
+        self.op_heads_store
+            .update_op_heads(self.operation().parent_ids(), self.operation().id());
+        self.repo
     }
 
-    pub fn leave_unpublished(mut self) -> Arc<ReadonlyRepo> {
-        let data = self.data.take().unwrap();
-        let repo = self
-            .repo_loader
-            .create_from(data.operation, data.view, data.index);
-        self.closed = true;
-        repo
-    }
-}
-
-impl Drop for UnpublishedOperation {
-    fn drop(&mut self) {
-        if !self.closed && !std::thread::panicking() {
-            eprintln!("BUG: UnpublishedOperation was dropped without being closed.");
-        }
+    pub fn leave_unpublished(self) -> Arc<ReadonlyRepo> {
+        self.repo
     }
 }