refactor(content_hash): expose content_hash without `keys_being_cal…

…culated` parameter Callers will always initialize this with an empty hash set, so hide that implementation detail.
twitter · Sep 6, 2022 · 7d527f3 · 7d527f3
1 parent 39a1e9b
commit 7d527f3
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 22 deletions.
diff --git a/focus/commands/examples/calc_invalidation_rate.rs b/focus/commands/examples/calc_invalidation_rate.rs
@@ -8,7 +8,7 @@
 //! ```
 
 use std::{
-    collections::{HashMap, HashSet},
+    collections::HashMap,
     ops::Deref,
     path::PathBuf,
     sync::{
@@ -19,7 +19,7 @@ use std::{
 
 use clap::Parser;
 use focus_internals::{
-    index::{content_hash_dependency_key, ContentHash, DependencyKey, HashContext},
+    index::{content_hash, ContentHash, DependencyKey, HashContext},
     model::{repo::Repo, selection::Project},
     target::{Target, TargetSet},
 };
@@ -165,9 +165,7 @@ fn main() -> anyhow::Result<()> {
                 .iter()
                 .map(|target| {
                     let dep_key = DependencyKey::from(target.clone());
-                    let hash =
-                        content_hash_dependency_key(&hash_context, &dep_key, &mut HashSet::new())
-                            .unwrap();
+                    let hash = content_hash(&hash_context, &dep_key).unwrap();
                     ((*commit_oid, target), hash)
                 })
                 .collect::<Vec<_>>()

diff --git a/focus/internals/benches/bench_content_hash.rs b/focus/internals/benches/bench_content_hash.rs
@@ -1,15 +1,14 @@
 // Copyright 2022 Twitter, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::collections::HashSet;
 use std::path::PathBuf;
 use std::sync::Arc;
 
 use content_addressed_cache::RocksDBCache;
 use criterion::{criterion_group, criterion_main, Criterion};
 use focus_internals::index::{
-    content_hash_dependency_key, ContentHash, DependencyKey, DependencyValue, HashContext,
-    ObjectDatabase, RocksDBMemoizationCacheExt, SimpleGitOdb,
+    content_hash, ContentHash, DependencyKey, DependencyValue, HashContext, ObjectDatabase,
+    RocksDBMemoizationCacheExt, SimpleGitOdb,
 };
 use focus_internals::model::repo::Repo;
 use focus_util::app::App;
@@ -19,7 +18,7 @@ fn content_hash_dependency_keys(ctx: &HashContext, dep_keys: &[DependencyKey]) -
         .iter()
         .map(|dep_key| {
             let dep_key = DependencyKey::DummyForTesting(Box::new(dep_key.clone()));
-            content_hash_dependency_key(ctx, &dep_key, &mut HashSet::new()).unwrap()
+            content_hash(ctx, &dep_key).unwrap()
         })
         .collect::<Vec<_>>()
 }

diff --git a/focus/internals/src/lib/index/content_hash.rs b/focus/internals/src/lib/index/content_hash.rs
@@ -107,7 +107,11 @@ impl std::fmt::Debug for HashContext<'_> {
 
 /// Compute a content-addressable hash for the provided [`DependencyKey`] using
 /// the context in `ctx`.
-pub fn content_hash_dependency_key(
+pub fn content_hash(ctx: &HashContext, key: &DependencyKey) -> anyhow::Result<ContentHash> {
+    content_hash_dependency_key(ctx, key, &mut HashSet::new())
+}
+
+fn content_hash_dependency_key(
     ctx: &HashContext,
     key: &DependencyKey,
     keys_being_calculated: &mut HashSet<DependencyKey>,

diff --git a/focus/internals/src/lib/index/mod.rs b/focus/internals/src/lib/index/mod.rs
@@ -11,7 +11,7 @@ mod content_hash;
 mod dependency_graph;
 mod object_database;
 
-pub use content_hash::{content_hash_dependency_key, ContentHash, HashContext};
+pub use content_hash::{content_hash, ContentHash, HashContext};
 pub use dependency_graph::{
     get_files_to_materialize, update_object_database_from_resolution, DependencyKey,
     DependencyValue, PathsToMaterializeResult,

diff --git a/focus/internals/src/lib/index/object_database.rs b/focus/internals/src/lib/index/object_database.rs
@@ -1,11 +1,10 @@
 // Copyright 2022 Twitter, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::collections::HashSet;
 use std::time::Duration;
 
 use super::content_hash::HashContext;
-use super::{content_hash_dependency_key, ContentHash, DependencyKey, DependencyValue};
+use super::{content_hash, ContentHash, DependencyKey, DependencyValue};
 use anyhow::Context;
 use content_addressed_cache::Cache;
 use tracing::{debug, error, info, info_span, warn};
@@ -46,7 +45,7 @@ impl<T: Cache> ObjectDatabase for T {
         ctx: &HashContext,
         key: &DependencyKey,
     ) -> anyhow::Result<(ContentHash, Option<DependencyValue>)> {
-        let hash = content_hash_dependency_key(ctx, key, &mut HashSet::new())?;
+        let hash = content_hash(ctx, key)?;
         let result = self.get_direct(&hash)?;
         Ok((hash, result))
     }
@@ -66,7 +65,7 @@ impl<T: Cache> ObjectDatabase for T {
         key: &DependencyKey,
         value: DependencyValue,
     ) -> anyhow::Result<()> {
-        let hash = content_hash_dependency_key(ctx, key, &mut HashSet::new())?;
+        let hash = content_hash(ctx, key)?;
         debug!(?hash, ?value, "Inserting entry into object database");
         let payload = serde_json::to_vec(&value).context("serializing DependencyValue as JSON")?;
 
@@ -114,7 +113,7 @@ impl RocksDBMemoizationCacheExt for RocksDBCache {
 #[cfg(test)]
 /// Testing utilities.
 pub mod testing {
-    use crate::index::content_hash_dependency_key;
+    use crate::index::content_hash;
 
     use super::*;
 
@@ -146,7 +145,7 @@ pub mod testing {
             ctx: &HashContext,
             key: &DependencyKey,
         ) -> anyhow::Result<(ContentHash, Option<DependencyValue>)> {
-            let hash = content_hash_dependency_key(ctx, key, &mut HashSet::new())?;
+            let hash = content_hash(ctx, key)?;
             let dep_value = self.get_direct(&hash)?;
             Ok((hash, dep_value))
         }
@@ -163,7 +162,7 @@ pub mod testing {
             key: &DependencyKey,
             value: DependencyValue,
         ) -> anyhow::Result<()> {
-            let hash = content_hash_dependency_key(ctx, key, &mut HashSet::new())?;
+            let hash = content_hash(ctx, key)?;
             debug!(?hash, ?value, "Inserting entry into object database");
 
             let mut entries = self.entries.lock().expect("poisoned mutex");
@@ -210,7 +209,7 @@ impl ObjectDatabase for SimpleGitOdb<'_> {
         ctx: &HashContext,
         key: &DependencyKey,
     ) -> anyhow::Result<(ContentHash, Option<DependencyValue>)> {
-        let hash = content_hash_dependency_key(ctx, key, &mut HashSet::new())?;
+        let hash = content_hash(ctx, key)?;
         let result = self.get_direct(&hash)?;
         Ok((hash, result))
     }
@@ -255,7 +254,7 @@ impl ObjectDatabase for SimpleGitOdb<'_> {
         key: &DependencyKey,
         value: DependencyValue,
     ) -> anyhow::Result<()> {
-        let ContentHash(key_oid) = content_hash_dependency_key(ctx, key, &mut HashSet::new())?;
+        let ContentHash(key_oid) = content_hash(ctx, key)?;
         let payload = serde_json::to_vec(&value).context("serializing DependencyValue as JSON")?;
         let value_oid = ctx
             .repo

diff --git a/focus/operations/src/index.rs b/focus/operations/src/index.rs
@@ -16,7 +16,7 @@ use focus_util::paths::assert_focused_repo;
 use tracing::{debug, debug_span, info};
 
 use focus_internals::index::{
-    content_hash_dependency_key, get_files_to_materialize, ContentHash, DependencyKey, HashContext,
+    content_hash, get_files_to_materialize, ContentHash, DependencyKey, HashContext,
     ObjectDatabase, PathsToMaterializeResult, RocksDBCache, RocksDBMemoizationCacheExt,
     FUNCTION_ID,
 };
@@ -191,7 +191,7 @@ pub fn hash(
     for target in targets {
         let target = Target::try_from(target.as_str())?;
         let dep_key = DependencyKey::from(target);
-        let hash = content_hash_dependency_key(&hash_context, &dep_key, &mut HashSet::new())?;
+        let hash = content_hash(&hash_context, &dep_key)?;
         println!("{hash} {dep_key:?}");
     }