From 7756cfa61b21aa3cb6648b7de8e976d4b1361fe8 Mon Sep 17 00:00:00 2001
From: Daniel Ploch <dploch@google.com>
Date: Wed, 14 Dec 2022 13:02:23 -0500
Subject: [PATCH] op_store: op store delegates should not implement the OpStore
 trait directly

---
 lib/src/legacy_thrift_op_store.rs | 16 +++-------------
 lib/src/proto_op_store.rs         | 28 +++++++++++++---------------
 2 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/lib/src/legacy_thrift_op_store.rs b/lib/src/legacy_thrift_op_store.rs
index cf63271c4..2eea7a6df 100644
--- a/lib/src/legacy_thrift_op_store.rs
+++ b/lib/src/legacy_thrift_op_store.rs
@@ -23,7 +23,7 @@ use thrift::protocol::{TCompactInputProtocol, TSerializable};
 
 use crate::backend::{CommitId, MillisSinceEpoch, Timestamp};
 use crate::op_store::{
-    BranchTarget, OpStore, OpStoreError, OpStoreResult, Operation, OperationId, OperationMetadata,
+    BranchTarget, OpStoreError, OpStoreResult, Operation, OperationId, OperationMetadata,
     RefTarget, View, ViewId, WorkspaceId,
 };
 use crate::simple_op_store_model;
@@ -59,30 +59,20 @@ impl ThriftOpStore {
     fn operation_path(&self, id: &OperationId) -> PathBuf {
         self.path.join("operations").join(id.hex())
     }
-}
 
-impl OpStore for ThriftOpStore {
-    fn read_view(&self, id: &ViewId) -> OpStoreResult<View> {
+    pub fn read_view(&self, id: &ViewId) -> OpStoreResult<View> {
         let path = self.view_path(id);
         let mut file = File::open(path).map_err(not_found_to_store_error)?;
         let thrift_view = read_thrift(&mut file)?;
         Ok(View::from(&thrift_view))
     }
 
-    fn write_view(&self, _view: &View) -> OpStoreResult<ViewId> {
-        panic!("ThriftOpStore is readonly");
-    }
-
-    fn read_operation(&self, id: &OperationId) -> OpStoreResult<Operation> {
+    pub fn read_operation(&self, id: &OperationId) -> OpStoreResult<Operation> {
         let path = self.operation_path(id);
         let mut file = File::open(path).map_err(not_found_to_store_error)?;
         let thrift_operation = read_thrift(&mut file)?;
         Ok(Operation::from(&thrift_operation))
     }
-
-    fn write_operation(&self, _operation: &Operation) -> OpStoreResult<OperationId> {
-        panic!("ThriftOpStore is readonly");
-    }
 }
 
 pub fn read_thrift<T: TSerializable>(input: &mut impl Read) -> OpStoreResult<T> {
diff --git a/lib/src/proto_op_store.rs b/lib/src/proto_op_store.rs
index 96cdad864..6b16a83b1 100644
--- a/lib/src/proto_op_store.rs
+++ b/lib/src/proto_op_store.rs
@@ -27,7 +27,7 @@ use crate::backend::{CommitId, MillisSinceEpoch, Timestamp};
 use crate::content_hash::blake2b_hash;
 use crate::file_util::persist_content_addressed_temp_file;
 use crate::op_store::{
-    BranchTarget, OpStore, OpStoreError, OpStoreResult, Operation, OperationId, OperationMetadata,
+    BranchTarget, OpStoreError, OpStoreResult, Operation, OperationId, OperationMetadata,
     RefTarget, View, ViewId, WorkspaceId,
 };
 
@@ -60,18 +60,8 @@ impl ProtoOpStore {
     fn operation_path(&self, id: &OperationId) -> PathBuf {
         self.path.join("operations").join(id.hex())
     }
-}
 
-fn not_found_to_store_error(err: std::io::Error) -> OpStoreError {
-    if err.kind() == ErrorKind::NotFound {
-        OpStoreError::NotFound
-    } else {
-        OpStoreError::from(err)
-    }
-}
-
-impl OpStore for ProtoOpStore {
-    fn read_view(&self, id: &ViewId) -> OpStoreResult<View> {
+    pub fn read_view(&self, id: &ViewId) -> OpStoreResult<View> {
         let path = self.view_path(id);
         let mut file = File::open(path).map_err(not_found_to_store_error)?;
 
@@ -79,7 +69,7 @@ impl OpStore for ProtoOpStore {
         Ok(view_from_proto(&proto))
     }
 
-    fn write_view(&self, view: &View) -> OpStoreResult<ViewId> {
+    pub fn write_view(&self, view: &View) -> OpStoreResult<ViewId> {
         let temp_file = NamedTempFile::new_in(&self.path)?;
 
         let proto = view_to_proto(view);
@@ -91,7 +81,7 @@ impl OpStore for ProtoOpStore {
         Ok(id)
     }
 
-    fn read_operation(&self, id: &OperationId) -> OpStoreResult<Operation> {
+    pub fn read_operation(&self, id: &OperationId) -> OpStoreResult<Operation> {
         let path = self.operation_path(id);
         let mut file = File::open(path).map_err(not_found_to_store_error)?;
 
@@ -99,7 +89,7 @@ impl OpStore for ProtoOpStore {
         Ok(operation_from_proto(&proto))
     }
 
-    fn write_operation(&self, operation: &Operation) -> OpStoreResult<OperationId> {
+    pub fn write_operation(&self, operation: &Operation) -> OpStoreResult<OperationId> {
         let temp_file = NamedTempFile::new_in(&self.path)?;
 
         let proto = operation_to_proto(operation);
@@ -112,6 +102,14 @@ impl OpStore for ProtoOpStore {
     }
 }
 
+fn not_found_to_store_error(err: std::io::Error) -> OpStoreError {
+    if err.kind() == ErrorKind::NotFound {
+        OpStoreError::NotFound
+    } else {
+        OpStoreError::from(err)
+    }
+}
+
 fn timestamp_to_proto(timestamp: &Timestamp) -> crate::protos::op_store::Timestamp {
     let mut proto = crate::protos::op_store::Timestamp::new();
     proto.millis_since_epoch = timestamp.timestamp.0;