mononoke_api: add create and delete bookmark methods

Summary: Add methods to create and delete bookmarks.

Reviewed By: StanislavGlebik

Differential Revision: D23288003

fbshipit-source-id: 5fca60254f00966478270e1a4447cc6a1b5a438e

eden/mononoke/mononoke_api/src/repo_write.rs

@@ -10,7 +10,9 @@ use std::ops::Deref;
 use crate::errors::MononokeError;
 use crate::repo::RepoContext;
 
+pub mod create_bookmark;
 pub mod create_changeset;
+pub mod delete_bookmark;
 pub mod move_bookmark;
 
 /// Describes the permissions model that is being used to determine if a write is

eden/mononoke/mononoke_api/src/repo_write/create_bookmark.rs

@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This software may be used and distributed according to the terms of the
+ * GNU General Public License version 2.
+ */
+
+use std::sync::Arc;
+
+use bookmarks::{BookmarkName, BookmarkUpdateReason};
+use metaconfig_types::BookmarkAttrs;
+use mononoke_types::ChangesetId;
+use reachabilityindex::LeastCommonAncestorsHint;
+
+use crate::errors::MononokeError;
+use crate::repo_write::{PermissionsModel, RepoWriteContext};
+
+impl RepoWriteContext {
+    /// Create a bookmark.
+    pub async fn create_bookmark(
+        &self,
+        bookmark: impl AsRef<str>,
+        target: ChangesetId,
+    ) -> Result<(), MononokeError> {
+        let bookmark = bookmark.as_ref();
+        self.check_method_permitted("create_bookmark")?;
+
+        let bookmark = BookmarkName::new(bookmark)?;
+        let bookmark_attrs = BookmarkAttrs::new(self.config().bookmarks.clone());
+
+        let lca_hint: Arc<dyn LeastCommonAncestorsHint> = self.skiplist_index().clone();
+
+        // Create the bookmark.
+        let mut op = bookmarks_movement::CreateBookmarkOp::new(
+            &bookmark,
+            target,
+            BookmarkUpdateReason::ApiRequest,
+        );
+
+        if let PermissionsModel::ServiceIdentity(service_identity) = &self.permissions_model {
+            op = op.for_service(service_identity, &self.config().source_control_service);
+        }
+
+        op.run(
+            self.ctx(),
+            self.blob_repo(),
+            &lca_hint,
+            &self.config().infinitepush,
+            &self.config().pushrebase,
+            &bookmark_attrs,
+            self.hook_manager().as_ref(),
+        )
+        .await?;
+
+        Ok(())
+    }
+}

eden/mononoke/mononoke_api/src/repo_write/delete_bookmark.rs

@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This software may be used and distributed according to the terms of the
+ * GNU General Public License version 2.
+ */
+
+use anyhow::Context;
+use bookmarks::{BookmarkName, BookmarkUpdateReason};
+use metaconfig_types::BookmarkAttrs;
+use mononoke_types::ChangesetId;
+
+use crate::errors::MononokeError;
+use crate::repo_write::{PermissionsModel, RepoWriteContext};
+
+impl RepoWriteContext {
+    /// Delete a bookmark.
+    pub async fn delete_bookmark(
+        &self,
+        bookmark: impl AsRef<str>,
+        old_target: Option<ChangesetId>,
+    ) -> Result<(), MononokeError> {
+        let bookmark = bookmark.as_ref();
+        self.check_method_permitted("delete_bookmark")?;
+
+        let bookmark = BookmarkName::new(bookmark)?;
+        let bookmark_attrs = BookmarkAttrs::new(self.config().bookmarks.clone());
+
+        // We need to find out where the bookmark currently points to in order
+        // to delete it.  Make sure to bypass any out-of-date caches.
+        let old_target = match old_target {
+            Some(old_target) => old_target,
+            None => self
+                .blob_repo()
+                .bookmarks()
+                .get(self.ctx().clone(), &bookmark)
+                .await
+                .context("Failed to fetch old bookmark target")?
+                .ok_or_else(|| {
+                    MononokeError::InvalidRequest(format!("bookmark '{}' does not exist", bookmark))
+                })?,
+        };
+
+        // Delete the bookmark.
+        let mut op = bookmarks_movement::DeleteBookmarkOp::new(
+            &bookmark,
+            old_target,
+            BookmarkUpdateReason::ApiRequest,
+        );
+
+        if let PermissionsModel::ServiceIdentity(service_identity) = &self.permissions_model {
+            op = op.for_service(service_identity, &self.config().source_control_service);
+        }
+
+        op.run(
+            self.ctx(),
+            self.blob_repo(),
+            &self.config().infinitepush,
+            &bookmark_attrs,
+        )
+        .await?;
+
+        Ok(())
+    }
+}

eden/mononoke/mononoke_api/src/repo_write/move_bookmark.rs

@@ -23,6 +23,7 @@ impl RepoWriteContext {
         &self,
         bookmark: impl AsRef<str>,
         target: ChangesetId,
+        old_target: Option<ChangesetId>,
         allow_non_fast_forward: bool,
     ) -> Result<(), MononokeError> {
         let bookmark = bookmark.as_ref();
@@ -33,15 +34,18 @@ impl RepoWriteContext {
 
         // We need to find out where the bookmark currently points to in order
         // to move it.  Make sure to bypass any out-of-date caches.
-        let old_target = self
-            .blob_repo()
-            .bookmarks()
-            .get(self.ctx().clone(), &bookmark)
-            .await
-            .context("Failed to fetch old bookmark target")?
-            .ok_or_else(|| {
-                MononokeError::InvalidRequest(format!("bookmark '{}' does not exist", bookmark))
-            })?;
+        let old_target = match old_target {
+            Some(old_target) => old_target,
+            None => self
+                .blob_repo()
+                .bookmarks()
+                .get(self.ctx().clone(), &bookmark)
+                .await
+                .context("Failed to fetch old bookmark target")?
+                .ok_or_else(|| {
+                    MononokeError::InvalidRequest(format!("bookmark '{}' does not exist", bookmark))
+                })?,
+        };
 
         let lca_hint: Arc<dyn LeastCommonAncestorsHint> = self.skiplist_index().clone();
 

eden/mononoke/mononoke_api/src/test/mod.rs

@@ -9,4 +9,4 @@ mod test_history;
 mod test_repo;
 mod test_repo_bookmarks;
 mod test_repo_create_changeset;
-mod test_repo_move_bookmark;
+mod test_repo_modify_bookmarks;

eden/mononoke/mononoke_api/src/test/test_repo_modify_bookmarks.rs

@@ -0,0 +1,184 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This software may be used and distributed according to the terms of the
+ * GNU General Public License version 2.
+ */
+
+use std::collections::BTreeMap;
+use std::sync::Arc;
+
+use anyhow::Result;
+use bookmarks::{BookmarkName, BookmarkUpdateLog, BookmarkUpdateReason, Freshness};
+use context::CoreContext;
+use fbinit::FacebookInit;
+use futures::stream::TryStreamExt;
+use mononoke_types::ChangesetId;
+use tests_utils::drawdag::create_from_dag;
+
+use crate::repo::{BookmarkFreshness, Repo, RepoContext};
+
+async fn init_repo(ctx: &CoreContext) -> Result<(RepoContext, BTreeMap<String, ChangesetId>)> {
+    let blob_repo = blobrepo_factory::new_memblob_empty(None)?;
+    let changesets = create_from_dag(
+        ctx,
+        &blob_repo,
+        r##"
+            A-B-C-D-E
+               \
+                F-G
+        "##,
+    )
+    .await?;
+    let mut txn = blob_repo.update_bookmark_transaction(ctx.clone());
+    txn.force_set(
+        &BookmarkName::new("trunk")?,
+        changesets["C"],
+        BookmarkUpdateReason::TestMove,
+        None,
+    )?;
+    txn.commit().await?;
+
+    let repo = Repo::new_test(ctx.clone(), blob_repo).await?;
+    let repo_ctx = RepoContext::new(ctx.clone(), Arc::new(repo)).await?;
+    Ok((repo_ctx, changesets))
+}
+
+#[fbinit::compat_test]
+async fn create_bookmark(fb: FacebookInit) -> Result<()> {
+    let ctx = CoreContext::test_mock(fb);
+    let (repo, changesets) = init_repo(&ctx).await?;
+    let repo = repo.write().await?;
+
+    // Can create public bookmarks on existing changesets (ancestors of trunk).
+    repo.create_bookmark("bookmark1", changesets["A"]).await?;
+    let bookmark1 = repo
+        .resolve_bookmark("bookmark1", BookmarkFreshness::MostRecent)
+        .await?
+        .expect("bookmark should be set");
+    assert_eq!(bookmark1.id(), changesets["A"]);
+
+    // Can create public bookmarks on other changesets (not ancestors of trunk).
+    repo.create_bookmark("bookmark2", changesets["F"]).await?;
+    let bookmark2 = repo
+        .resolve_bookmark("bookmark2", BookmarkFreshness::MostRecent)
+        .await?
+        .expect("bookmark should be set");
+    assert_eq!(bookmark2.id(), changesets["F"]);
+
+    // Can create scratch bookmarks.
+    repo.create_bookmark("scratch/bookmark3", changesets["G"])
+        .await?;
+    let bookmark3 = repo
+        .resolve_bookmark("scratch/bookmark3", BookmarkFreshness::MostRecent)
+        .await?
+        .expect("bookmark should be set");
+    assert_eq!(bookmark3.id(), changesets["G"]);
+
+    // F is now public.  G is not.
+    let stack = repo.stack(vec![changesets["G"]], 10).await?;
+    assert_eq!(stack.draft, vec![changesets["G"]]);
+    assert_eq!(stack.public, vec![changesets["F"]]);
+
+    Ok(())
+}
+
+#[fbinit::compat_test]
+async fn move_bookmark(fb: FacebookInit) -> Result<()> {
+    let ctx = CoreContext::test_mock(fb);
+    let (repo, changesets) = init_repo(&ctx).await?;
+    let repo = repo.write().await?;
+
+    repo.move_bookmark("trunk", changesets["E"], None, false)
+        .await?;
+    let trunk = repo
+        .resolve_bookmark("trunk", BookmarkFreshness::MostRecent)
+        .await?
+        .expect("bookmark should be set");
+    assert_eq!(trunk.id(), changesets["E"]);
+
+    // Attempt to move to a non-descendant commit without allowing
+    // non-fast-forward moves should fail.
+    assert!(repo
+        .move_bookmark("trunk", changesets["G"], None, false)
+        .await
+        .is_err());
+    repo.move_bookmark("trunk", changesets["G"], None, true)
+        .await?;
+    let trunk = repo
+        .resolve_bookmark("trunk", BookmarkFreshness::MostRecent)
+        .await?
+        .expect("bookmark should be set");
+    assert_eq!(trunk.id(), changesets["G"]);
+
+    // Check the bookmark moves created BookmarkLogUpdate entries
+    let entries = repo
+        .blob_repo()
+        .attribute_expected::<dyn BookmarkUpdateLog>()
+        .list_bookmark_log_entries(
+            ctx.clone(),
+            BookmarkName::new("trunk")?,
+            3,
+            None,
+            Freshness::MostRecent,
+        )
+        .map_ok(|(cs, rs, _ts)| (cs, rs)) // dropping timestamps
+        .try_collect::<Vec<_>>()
+        .await?;
+    assert_eq!(
+        entries,
+        vec![
+            (Some(changesets["G"]), BookmarkUpdateReason::ApiRequest),
+            (Some(changesets["E"]), BookmarkUpdateReason::ApiRequest),
+            (Some(changesets["C"]), BookmarkUpdateReason::TestMove),
+        ]
+    );
+
+    Ok(())
+}
+
+#[fbinit::compat_test]
+async fn delete_bookmark(fb: FacebookInit) -> Result<()> {
+    let ctx = CoreContext::test_mock(fb);
+    let (repo, changesets) = init_repo(&ctx).await?;
+    let repo = repo.write().await?;
+
+    repo.create_bookmark("bookmark1", changesets["A"]).await?;
+    repo.create_bookmark("bookmark2", changesets["F"]).await?;
+    repo.create_bookmark("scratch/bookmark3", changesets["G"])
+        .await?;
+
+    // Can delete public bookmarks.
+    repo.delete_bookmark("bookmark1", None).await?;
+    assert!(repo
+        .resolve_bookmark("bookmark1", BookmarkFreshness::MostRecent)
+        .await?
+        .is_none());
+
+    // Deleting a bookmark with the wrong old-target fails.
+    assert!(repo
+        .delete_bookmark("bookmark2", Some(changesets["E"]))
+        .await
+        .is_err());
+    let bookmark2 = repo
+        .resolve_bookmark("bookmark2", BookmarkFreshness::MostRecent)
+        .await?
+        .expect("bookmark should be set");
+    assert_eq!(bookmark2.id(), changesets["F"]);
+
+    // But with the right old-target succeeds.
+    repo.delete_bookmark("bookmark2", Some(changesets["F"]))
+        .await?;
+    assert!(repo
+        .resolve_bookmark("bookmark1", BookmarkFreshness::MostRecent)
+        .await?
+        .is_none());
+
+    // Can't delete scratch bookmarks.
+    assert!(repo
+        .delete_bookmark("scratch/bookmark3", None)
+        .await
+        .is_err());
+
+    Ok(())
+}

eden/mononoke/mononoke_api/src/test/test_repo_move_bookmark.rs

@@ -1,97 +0,0 @@
-/*
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This software may be used and distributed according to the terms of the
- * GNU General Public License version 2.
- */
-
-use std::collections::BTreeMap;
-use std::sync::Arc;
-
-use anyhow::Result;
-use bookmarks::{BookmarkName, BookmarkUpdateLog, BookmarkUpdateReason, Freshness};
-use context::CoreContext;
-use fbinit::FacebookInit;
-use futures::stream::TryStreamExt;
-use mononoke_types::ChangesetId;
-use tests_utils::drawdag::create_from_dag;
-
-use crate::repo::{BookmarkFreshness, Repo, RepoContext};
-
-async fn init_repo(ctx: &CoreContext) -> Result<(RepoContext, BTreeMap<String, ChangesetId>)> {
-    let blob_repo = blobrepo_factory::new_memblob_empty(None)?;
-    let changesets = create_from_dag(
-        ctx,
-        &blob_repo,
-        r##"
-            A-B-C-D-E
-               \
-                F-G
-        "##,
-    )
-    .await?;
-    let mut txn = blob_repo.update_bookmark_transaction(ctx.clone());
-    txn.force_set(
-        &BookmarkName::new("trunk")?,
-        changesets["C"],
-        BookmarkUpdateReason::TestMove,
-        None,
-    )?;
-    txn.commit().await?;
-
-    let repo = Repo::new_test(ctx.clone(), blob_repo).await?;
-    let repo_ctx = RepoContext::new(ctx.clone(), Arc::new(repo)).await?;
-    Ok((repo_ctx, changesets))
-}
-
-#[fbinit::compat_test]
-async fn move_bookmark(fb: FacebookInit) -> Result<()> {
-    let ctx = CoreContext::test_mock(fb);
-    let (repo, changesets) = init_repo(&ctx).await?;
-    let repo = repo.write().await?;
-
-    repo.move_bookmark("trunk", changesets["E"], false).await?;
-    let trunk = repo
-        .resolve_bookmark("trunk", BookmarkFreshness::MostRecent)
-        .await?
-        .expect("bookmark should be set");
-    assert_eq!(trunk.id(), changesets["E"]);
-
-    // Attempt to move to a non-descendant commit without allowing
-    // non-fast-forward moves should fail.
-    assert!(repo
-        .move_bookmark("trunk", changesets["G"], false)
-        .await
-        .is_err());
-    repo.move_bookmark("trunk", changesets["G"], true).await?;
-    let trunk = repo
-        .resolve_bookmark("trunk", BookmarkFreshness::MostRecent)
-        .await?
-        .expect("bookmark should be set");
-    assert_eq!(trunk.id(), changesets["G"]);
-
-    // Check the bookmark moves created BookmarkLogUpdate entries
-    let entries = repo
-        .blob_repo()
-        .attribute_expected::<dyn BookmarkUpdateLog>()
-        .list_bookmark_log_entries(
-            ctx.clone(),
-            BookmarkName::new("trunk")?,
-            3,
-            None,
-            Freshness::MostRecent,
-        )
-        .map_ok(|(cs, rs, _ts)| (cs, rs)) // dropping timestamps
-        .try_collect::<Vec<_>>()
-        .await?;
-    assert_eq!(
-        entries,
-        vec![
-            (Some(changesets["G"]), BookmarkUpdateReason::ApiRequest),
-            (Some(changesets["E"]), BookmarkUpdateReason::ApiRequest),
-            (Some(changesets["C"]), BookmarkUpdateReason::TestMove),
-        ]
-    );
-
-    Ok(())
-}

eden/mononoke/scs_server/src/methods/repo.rs

@@ -452,8 +452,13 @@ impl SourceControlServiceImpl {
             .ok_or_else(|| errors::commit_not_found(target.to_string()))?;
 
         // TODO(mbthomas): provide a way for the client to optionally specify the old value
-        repo.move_bookmark(bookmark, changeset.id(), params.allow_non_fast_forward_move)
-            .await?;
+        repo.move_bookmark(
+            bookmark,
+            changeset.id(),
+            None,
+            params.allow_non_fast_forward_move,
+        )
+        .await?;
         Ok(thrift::RepoMoveBookmarkResponse {})
     }