mononoke: remove update_mapping method

Summary:
update_mapping method was silently setting current mapping version. This was
very error-prone and we had a replacement update_mapping_with_version. This
diff removes update_mapping completely.

Reviewed By: krallin

Differential Revision: D24360105

fbshipit-source-id: 57761f4279f75032e9d4ec88a45e5199e250247a

eden/mononoke/commit_rewriting/commit_validator/src/validation.rs

@@ -1399,7 +1399,7 @@ pub async fn validate_entry(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use cross_repo_sync::update_mapping;
+    use cross_repo_sync::update_mapping_with_version;
     use cross_repo_sync_test_utils::init_small_large_repo;
     use fbinit::FacebookInit;
     use maplit::hashmap;
@@ -1462,7 +1462,13 @@ mod tests {
             })
             .collect();
 
-        update_mapping(ctx.clone(), commit_mapping, &small_to_large_commit_syncer).await?;
+        update_mapping_with_version(
+            ctx.clone(),
+            commit_mapping,
+            &small_to_large_commit_syncer,
+            &small_to_large_commit_syncer.get_current_version(&ctx)?,
+        )
+        .await?;
         let large_repo = Large(large_repo.clone());
         let small_repo = Small(small_repo.clone());
 

eden/mononoke/commit_rewriting/cross_repo_sync/src/lib.rs

@@ -845,10 +845,12 @@ where
                 let frozen_cs_id = frozen.get_changeset_id();
                 upload_commits(ctx.clone(), vec![frozen], source_repo, target_repo.clone()).await?;
 
-                update_mapping(
+                let version_name = self.get_current_version(&ctx)?;
+                update_mapping_with_version(
                     ctx.clone(),
                     hashmap! { source_cs_id => frozen_cs_id },
                     &self,
+                    &version_name,
                 )
                 .await?;
                 Ok(Some(frozen_cs_id))
@@ -1499,31 +1501,6 @@ pub async fn upload_commits(
     Ok(())
 }
 
-// TODO(stash, ikostia) - replace all usages with update_mapping_with_version and
-// remove this function
-pub async fn update_mapping<'a, M: SyncedCommitMapping + Clone + 'static>(
-    ctx: CoreContext,
-    mapped: HashMap<ChangesetId, ChangesetId>,
-    syncer: &'a CommitSyncer<M>,
-) -> Result<(), Error> {
-    // TODO(stash): we shouldn't always use current version, but rather pass the actual version
-    // that was used to remap a commit
-    let version_name = syncer.get_current_version(&ctx)?;
-    let entries: Vec<_> = mapped
-        .into_iter()
-        .map(|(from, to)| {
-            cloned!(version_name);
-            create_synced_commit_mapping_entry(from, to, &syncer.repos, Some(version_name))
-        })
-        .collect();
-    syncer
-        .mapping
-        .add_bulk(ctx.clone(), entries)
-        .compat()
-        .await?;
-    Ok(())
-}
-
 pub async fn update_mapping_with_version<'a, M: SyncedCommitMapping + Clone + 'static>(
     ctx: CoreContext,
     mapped: HashMap<ChangesetId, ChangesetId>,

eden/mononoke/commit_rewriting/cross_repo_sync/test_utils/lib.rs

@@ -19,7 +19,7 @@ use context::CoreContext;
 use cross_repo_sync::{
     rewrite_commit,
     types::{Source, Target},
-    update_mapping, upload_commits, CandidateSelectionHint, CommitSyncDataProvider,
+    update_mapping_with_version, upload_commits, CandidateSelectionHint, CommitSyncDataProvider,
     CommitSyncRepos, CommitSyncer, SyncData, Syncers,
 };
 use futures::{compat::Future01CompatExt, FutureExt, TryFutureExt};
@@ -299,10 +299,11 @@ pub async fn init_small_large_repo(
         .set_to(large_master_bcs_id)
         .await?;
 
-    update_mapping(
+    update_mapping_with_version(
         ctx.clone(),
         hashmap! { small_master_bcs_id => large_master_bcs_id},
         &small_to_large_commit_syncer,
+        &small_to_large_commit_syncer.get_current_version(&ctx)?,
     )
     .await?;
 

eden/mononoke/commit_rewriting/megarepo/tool/sync_diamond_merge.rs

@@ -19,8 +19,8 @@ use bookmarks::{BookmarkName, BookmarkUpdateReason};
 use cloned::cloned;
 use context::CoreContext;
 use cross_repo_sync::{
-    create_commit_syncers, rewrite_commit, update_mapping, upload_commits, CandidateSelectionHint,
-    CommitSyncOutcome, CommitSyncer, Syncers,
+    create_commit_syncers, rewrite_commit, update_mapping_with_version, upload_commits,
+    CandidateSelectionHint, CommitSyncOutcome, CommitSyncer, Syncers,
 };
 use futures::{
     compat::Future01CompatExt,
@@ -167,10 +167,11 @@ pub async fn do_sync_diamond_merge(
     info!(ctx.logger(), "uploading merge commit {}", new_merge_cs_id);
     upload_commits(ctx.clone(), vec![rewritten], small_repo, large_repo.clone()).await?;
 
-    update_mapping(
+    update_mapping_with_version(
         ctx.clone(),
         hashmap! {small_merge_cs_id => new_merge_cs_id},
         &syncers.small_to_large,
+        &syncers.small_to_large.get_current_version(&ctx)?,
     )
     .await?;