mononoke: make version mapping non-optional in WorkingCopyEquivalence

Summary:
In the next diff we are going to refactor CommitSyncOutcome::NotSyncCandidate
to have a version, which would mean that the version should always be present
even for NotSyncCandidate

So in this diff let's make mapping version non-optional. Note that at the moment our
prod tables have some of the mapping entries with a non-empty version - I'm
going to fix it before landing this diff.

Reviewed By: mitrandir77

Differential Revision: D32428740

fbshipit-source-id: be9f8b104a14497bf5a524c68c0fd53beba22ee1

eden/mononoke/commit_rewriting/backsyncer/src/tests.rs

@@ -1856,6 +1856,7 @@ async fn preserve_premerge_commit(
         bcs_id
     );
 
+    let version = CommitSyncConfigVersion("noop".to_string());
     // Doesn't matter what mover to use - we are going to preserve the commit anyway
     let small_to_large_sync_config = {
         let repos = CommitSyncRepos::SmallToLarge {
@@ -1868,7 +1869,6 @@ async fn preserve_premerge_commit(
         let bookmark_renamer_type = BookmarkRenamerType::Noop;
         let mover_type = MoverType::Noop;
 
-        let version = CommitSyncConfigVersion("noop".to_string());
         let version_config = CommitSyncConfig {
             large_repo_id: large_repo.get_repoid(),
             common_pushrebase_bookmarks: vec![BookmarkName::new("master")?],
@@ -1879,7 +1879,7 @@ async fn preserve_premerge_commit(
         };
 
         lv_cfg_src.add_config(version_config);
-        lv_cfg_src.add_current_version(version);
+        lv_cfg_src.add_current_version(version.clone());
         let common = bookmark_renamer_type
             .get_common_repo_config(small_repo.get_repoid(), large_repo.get_repoid());
         lv_cfg_src.add_common_config(common);
@@ -1907,7 +1907,7 @@ async fn preserve_premerge_commit(
                     large_bcs_id: bcs_id,
                     small_repo_id: another_repo_id,
                     small_bcs_id: None,
-                    version_name: None,
+                    version_name: Some(version.clone()),
                 },
             )
             .await?;

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

@@ -227,19 +227,9 @@ pub async fn get_plural_commit_sync_outcome<'a, M: SyncedCommitMapping>(
         Some(WorkingCopyEquivalence::NoWorkingCopy(_version)) => {
             Ok(Some(PluralCommitSyncOutcome::NotSyncCandidate))
         }
-        Some(WorkingCopyEquivalence::WorkingCopy(cs_id, maybe_version)) => {
-            let version = maybe_version.ok_or_else(|| {
-                anyhow!(
-                    "no sync commit version specified for equivalent working copy {} -> {} (source repo {}, target repo {})",
-                    source_cs_id.0, cs_id,
-                    source_repo_id,
-                    target_repo_id,
-                )
-            })?;
-            Ok(Some(
-                PluralCommitSyncOutcome::EquivalentWorkingCopyAncestor(cs_id, version),
-            ))
-        }
+        Some(WorkingCopyEquivalence::WorkingCopy(cs_id, version)) => Ok(Some(
+            PluralCommitSyncOutcome::EquivalentWorkingCopyAncestor(cs_id, version),
+        )),
     }
 }
 

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

@@ -11,7 +11,7 @@ use sql::{Connection, Transaction};
 use sql_construct::{SqlConstruct, SqlConstructFromMetadataDatabaseConfig};
 use sql_ext::SqlConnections;
 
-use anyhow::Error;
+use anyhow::{anyhow, Error};
 use async_trait::async_trait;
 use auto_impl::auto_impl;
 use context::{CoreContext, PerfCounterType};
@@ -163,9 +163,9 @@ pub struct EquivalentWorkingCopyEntry {
 pub enum WorkingCopyEquivalence {
     /// There's no matching working copy. It can happen if a pre-big-merge commit from one small
     /// repo is mapped into another small repo
-    NoWorkingCopy(Option<CommitSyncConfigVersion>),
+    NoWorkingCopy(CommitSyncConfigVersion),
     /// ChangesetId of matching working copy and CommitSyncConfigVersion that was used for mapping
-    WorkingCopy(ChangesetId, Option<CommitSyncConfigVersion>),
+    WorkingCopy(ChangesetId, CommitSyncConfigVersion),
 }
 
 #[async_trait]
@@ -515,6 +515,7 @@ impl SyncedCommitMapping for SqlSyncedCommitMapping {
                     WorkingCopy(wc, mapping) => (Some(wc), mapping),
                     NoWorkingCopy(mapping) => (None, mapping),
                 };
+                let expected_version = Some(expected_version);
                 if (expected_bcs_id != small_bcs_id) || (expected_version != version_name) {
                     let err = ErrorKind::InconsistentWorkingCopyEntry {
                         expected_bcs_id,
@@ -573,18 +574,22 @@ impl SyncedCommitMapping for SqlSyncedCommitMapping {
                     maybe_mapping,
                 ) = row;
 
+                let mapping = maybe_mapping.ok_or_else(|| {
+                    anyhow!(
+                        "unexpected empty mapping for {}, {}->{}",
+                        source_bcs_id,
+                        source_repo_id,
+                        target_repo_id
+                    )
+                })?;
                 if target_repo_id == large_repo_id {
-                    Some(WorkingCopyEquivalence::WorkingCopy(
-                        large_bcs_id,
-                        maybe_mapping,
-                    ))
+                    Some(WorkingCopyEquivalence::WorkingCopy(large_bcs_id, mapping))
                 } else {
                     match maybe_small_bcs_id {
-                        Some(small_bcs_id) => Some(WorkingCopyEquivalence::WorkingCopy(
-                            small_bcs_id,
-                            maybe_mapping,
-                        )),
-                        None => Some(WorkingCopyEquivalence::NoWorkingCopy(maybe_mapping)),
+                        Some(small_bcs_id) => {
+                            Some(WorkingCopyEquivalence::WorkingCopy(small_bcs_id, mapping))
+                        }
+                        None => Some(WorkingCopyEquivalence::NoWorkingCopy(mapping)),
                     }
                 }
             }

eden/mononoke/commit_rewriting/synced_commit_mapping/test/main.rs

@@ -57,7 +57,7 @@ async fn add_and_get<M: SyncedCommitMapping>(fb: FacebookInit, mapping: M) {
         res,
         Some(WorkingCopyEquivalence::WorkingCopy(
             bonsai::TWOS_CSID,
-            Some(version_name.clone()),
+            version_name.clone(),
         ))
     );
 
@@ -87,7 +87,7 @@ async fn add_and_get<M: SyncedCommitMapping>(fb: FacebookInit, mapping: M) {
         res,
         Some(WorkingCopyEquivalence::WorkingCopy(
             bonsai::FOURS_CSID,
-            Some(version_name.clone()),
+            version_name.clone(),
         ))
     );
 
@@ -169,7 +169,7 @@ async fn equivalent_working_copy<M: SyncedCommitMapping>(fb: FacebookInit, mappi
         res,
         Some(WorkingCopyEquivalence::WorkingCopy(
             bonsai::TWOS_CSID,
-            Some(version_name)
+            version_name
         ))
     );
 
@@ -187,13 +187,6 @@ async fn equivalent_working_copy<M: SyncedCommitMapping>(fb: FacebookInit, mappi
         .expect("Failed to insert working copy");
     assert_eq!(result, true);
 
-    let res = mapping
-        .get_equivalent_working_copy(&ctx, REPO_ZERO, bonsai::THREES_CSID, REPO_ONE)
-        .await
-        .expect("get equivalent wc failed, should succeed");
-
-    assert_eq!(res, Some(WorkingCopyEquivalence::NoWorkingCopy(None)));
-
     let should_fail = EquivalentWorkingCopyEntry {
         large_repo_id: REPO_ZERO,
         large_bcs_id: bonsai::THREES_CSID,