From 837ac15052cdf01b80d008df3a9f8c7b997b3356 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 4 Jan 2024 11:51:58 +0900 Subject: [PATCH] op_store: add resolve_operation_id_prefix() trait method that uses readdir() The OpStore backends should have a better way to look up operation by id than traversing from the op heads. The added method is similar to the commit Index one, but returns an OpStoreResult because the backend operation can fail. FWIW, if we want .shortest() in the op log template, we'll probably need a trait method that returns an OpIndex instead. --- lib/src/op_store.rs | 8 +++++- lib/src/op_walk.rs | 42 +++++++++--------------------- lib/src/simple_op_store.rs | 53 +++++++++++++++++++++++++++++++++++--- 3 files changed, 69 insertions(+), 34 deletions(-) diff --git a/lib/src/op_store.rs b/lib/src/op_store.rs index 795f85ede..8533a192d 100644 --- a/lib/src/op_store.rs +++ b/lib/src/op_store.rs @@ -25,7 +25,7 @@ use thiserror::Error; use crate::backend::{CommitId, Timestamp}; use crate::content_hash::ContentHash; use crate::merge::Merge; -use crate::object_id::{id_type, ObjectId}; +use crate::object_id::{id_type, HexPrefix, ObjectId, PrefixResolution}; content_hash! { #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash)] @@ -418,6 +418,12 @@ pub trait OpStore: Send + Sync + Debug { fn read_operation(&self, id: &OperationId) -> OpStoreResult; fn write_operation(&self, contents: &Operation) -> OpStoreResult; + + /// Resolves an unambiguous operation ID prefix. + fn resolve_operation_id_prefix( + &self, + prefix: &HexPrefix, + ) -> OpStoreResult>; } #[cfg(test)] diff --git a/lib/src/op_walk.rs b/lib/src/op_walk.rs index adae474f0..09f17869c 100644 --- a/lib/src/op_walk.rs +++ b/lib/src/op_walk.rs @@ -22,7 +22,7 @@ use std::sync::Arc; use itertools::Itertools as _; use thiserror::Error; -use crate::object_id::HexPrefix; +use crate::object_id::{HexPrefix, PrefixResolution}; use crate::op_heads_store::{OpHeadResolutionError, OpHeadsStore}; use crate::op_store::{OpStore, OpStoreError, OpStoreResult, OperationId}; use crate::operation::Operation; @@ -110,7 +110,7 @@ fn resolve_single_op( .transpose()?; let mut operation = match op_symbol { "@" => get_current_op(), - s => resolve_single_op_from_store(op_store, op_heads_store, s), + s => resolve_single_op_from_store(op_store, s), }?; for c in op_postfix.chars() { let mut neighbor_ops = match c { @@ -129,7 +129,6 @@ fn resolve_single_op( fn resolve_single_op_from_store( op_store: &Arc, - op_heads_store: &dyn OpHeadsStore, op_str: &str, ) -> Result { if op_str.is_empty() { @@ -137,34 +136,17 @@ fn resolve_single_op_from_store( } let prefix = HexPrefix::new(op_str) .ok_or_else(|| OpsetResolutionError::InvalidIdPrefix(op_str.to_owned()))?; - if let Some(binary_op_id) = prefix.as_full_bytes() { - let op_id = OperationId::from_bytes(binary_op_id); - match op_store.read_operation(&op_id) { - Ok(operation) => { - return Ok(Operation::new(op_store.clone(), op_id, operation)); - } - Err(OpStoreError::ObjectNotFound { .. }) => { - // Fall through - } - Err(err) => { - return Err(OpsetEvaluationError::OpStore(err)); - } + match op_store.resolve_operation_id_prefix(&prefix)? { + PrefixResolution::NoMatch => { + Err(OpsetResolutionError::NoSuchOperation(op_str.to_owned()).into()) + } + PrefixResolution::SingleMatch(op_id) => { + let data = op_store.read_operation(&op_id)?; + Ok(Operation::new(op_store.clone(), op_id, data)) + } + PrefixResolution::AmbiguousMatch => { + Err(OpsetResolutionError::AmbiguousIdPrefix(op_str.to_owned()).into()) } - } - - // TODO: Extract to OpStore method where IDs can be resolved without loading - // all operation data? - let head_ops = get_current_head_ops(op_store, op_heads_store)?; - let mut matches: Vec<_> = walk_ancestors(&head_ops) - .filter_ok(|op| prefix.matches(op.id())) - .take(2) - .try_collect()?; - if matches.is_empty() { - Err(OpsetResolutionError::NoSuchOperation(op_str.to_owned()).into()) - } else if matches.len() == 1 { - Ok(matches.pop().unwrap()) - } else { - Err(OpsetResolutionError::AmbiguousIdPrefix(op_str.to_owned()).into()) } } diff --git a/lib/src/simple_op_store.rs b/lib/src/simple_op_store.rs index 645511a46..c5c99914c 100644 --- a/lib/src/simple_op_store.rs +++ b/lib/src/simple_op_store.rs @@ -16,9 +16,9 @@ use std::collections::BTreeMap; use std::fmt::Debug; -use std::fs; use std::io::{ErrorKind, Write}; use std::path::{Path, PathBuf}; +use std::{fs, io}; use prost::Message; use tempfile::NamedTempFile; @@ -26,15 +26,18 @@ use thiserror::Error; use crate::backend::{CommitId, MillisSinceEpoch, Timestamp}; use crate::content_hash::blake2b_hash; -use crate::file_util::persist_content_addressed_temp_file; +use crate::file_util::{persist_content_addressed_temp_file, IoResultExt as _}; use crate::merge::Merge; -use crate::object_id::ObjectId; +use crate::object_id::{HexPrefix, ObjectId, PrefixResolution}; use crate::op_store::{ OpStore, OpStoreError, OpStoreResult, Operation, OperationId, OperationMetadata, RefTarget, RemoteRef, RemoteRefState, RemoteView, View, ViewId, WorkspaceId, }; use crate::{git, op_store}; +// BLAKE2b-512 hash length in bytes +const OPERATION_ID_LENGTH: usize = 64; + #[derive(Debug, Error)] #[error("Failed to read {kind} with ID {id}: {err}")] struct DecodeError { @@ -148,6 +151,50 @@ impl OpStore for SimpleOpStore { .map_err(|err| io_to_write_error(err, "operation"))?; Ok(id) } + + fn resolve_operation_id_prefix( + &self, + prefix: &HexPrefix, + ) -> OpStoreResult> { + let op_dir = self.path.join("operations"); + let find = || -> io::Result<_> { + let hex_prefix = prefix.hex(); + if hex_prefix.len() == OPERATION_ID_LENGTH * 2 { + // Fast path for full-length ID + if op_dir.join(hex_prefix).try_exists()? { + let id = OperationId::from_bytes(prefix.as_full_bytes().unwrap()); + return Ok(PrefixResolution::SingleMatch(id)); + } else { + return Ok(PrefixResolution::NoMatch); + } + } + + let mut matched = None; + for entry in op_dir.read_dir()? { + let Ok(name) = entry?.file_name().into_string() else { + continue; // Skip invalid UTF-8 + }; + if !name.starts_with(&hex_prefix) { + continue; + } + let Ok(id) = OperationId::try_from_hex(&name) else { + continue; // Skip invalid hex + }; + if matched.is_some() { + return Ok(PrefixResolution::AmbiguousMatch); + } + matched = Some(id); + } + if let Some(id) = matched { + Ok(PrefixResolution::SingleMatch(id)) + } else { + Ok(PrefixResolution::NoMatch) + } + }; + find() + .context(&op_dir) + .map_err(|err| OpStoreError::Other(err.into())) + } } fn io_to_read_error(err: std::io::Error, id: &impl ObjectId) -> OpStoreError {