SERVER-28211 add StorageInterface::deleteById()

5 files changed, 94 insertions, 18 deletions

diff --git a/src/mongo/db/repl/storage_interface.h b/src/mongo/db/repl/storage_interface.h
index 2cc34104b83..65a33a72a79 100644
--- a/src/mongo/db/repl/storage_interface.h
+++ b/src/mongo/db/repl/storage_interface.h
@@ -272,6 +272,14 @@ public:
                                          const BSONElement& idKey) = 0;
 
     /**
+     * Deletes a single document in the collection referenced by the specified _id.
+     * Returns deleted document on success.
+     */
+    virtual StatusWith<BSONObj> deleteById(OperationContext* opCtx,
+                                           const NamespaceString& nss,
+                                           const BSONElement& idKey) = 0;
+
+    /**
      * Updates a single document in the collection referenced by the specified _id.
      * The document is located by looking up "idKey" in the id index.
      * "update" represents the replacement document or list of requested modifications to be applied
diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp
index a51143aaedc..d6f557f2ac4 100644
--- a/src/mongo/db/repl/storage_interface_impl.cpp
+++ b/src/mongo/db/repl/storage_interface_impl.cpp
@@ -590,6 +590,31 @@ StatusWith<std::vector<BSONObj>> _findOrDeleteDocuments(
     MONGO_UNREACHABLE;
 }
 
+StatusWith<BSONObj> _findOrDeleteById(OperationContext* opCtx,
+                                      const NamespaceString& nss,
+                                      const BSONElement& idKey,
+                                      FindDeleteMode mode) {
+    auto wrappedIdKey = idKey.wrap("");
+    auto result = _findOrDeleteDocuments(opCtx,
+                                         nss,
+                                         kIdIndexName,
+                                         StorageInterface::ScanDirection::kForward,
+                                         wrappedIdKey,
+                                         wrappedIdKey,
+                                         BoundInclusion::kIncludeBothStartAndEndKeys,
+                                         1U,
+                                         mode);
+    if (!result.isOK()) {
+        return result.getStatus();
+    }
+    const auto& docs = result.getValue();
+    if (docs.empty()) {
+        return {ErrorCodes::NoSuchKey, str::stream() << "No document found with _id: " << idKey};
+    }
+
+    return docs.front();
+}
+
 }  // namespace
 
 StatusWith<std::vector<BSONObj>> StorageInterfaceImpl::findDocuments(
@@ -633,25 +658,13 @@ StatusWith<std::vector<BSONObj>> StorageInterfaceImpl::deleteDocuments(
 StatusWith<BSONObj> StorageInterfaceImpl::findById(OperationContext* opCtx,
                                                    const NamespaceString& nss,
                                                    const BSONElement& idKey) {
-    auto wrappedIdKey = idKey.wrap("");
-    auto result = _findOrDeleteDocuments(opCtx,
-                                         nss,
-                                         kIdIndexName,
-                                         ScanDirection::kForward,
-                                         wrappedIdKey,
-                                         wrappedIdKey,
-                                         BoundInclusion::kIncludeBothStartAndEndKeys,
-                                         1U,
-                                         FindDeleteMode::kFind);
-    if (!result.isOK()) {
-        return result.getStatus();
-    }
-    const auto& docs = result.getValue();
-    if (docs.empty()) {
-        return {ErrorCodes::NoSuchKey, str::stream() << "No document found with _id: " << idKey};
-    }
+    return _findOrDeleteById(opCtx, nss, idKey, FindDeleteMode::kFind);
+}
 
-    return docs.front();
+StatusWith<BSONObj> StorageInterfaceImpl::deleteById(OperationContext* opCtx,
+                                                     const NamespaceString& nss,
+                                                     const BSONElement& idKey) {
+    return _findOrDeleteById(opCtx, nss, idKey, FindDeleteMode::kDelete);
 }
 
 namespace {
diff --git a/src/mongo/db/repl/storage_interface_impl.h b/src/mongo/db/repl/storage_interface_impl.h
index b5769c69cc1..76bbc064e0f 100644
--- a/src/mongo/db/repl/storage_interface_impl.h
+++ b/src/mongo/db/repl/storage_interface_impl.h
@@ -121,6 +121,10 @@ public:
                                  const NamespaceString& nss,
                                  const BSONElement& idKey) override;
 
+    StatusWith<BSONObj> deleteById(OperationContext* opCtx,
+                                   const NamespaceString& nss,
+                                   const BSONElement& idKey) override;
+
     Status upsertById(OperationContext* opCtx,
                       const NamespaceString& nss,
                       const BSONElement& idKey,
diff --git a/src/mongo/db/repl/storage_interface_impl_test.cpp b/src/mongo/db/repl/storage_interface_impl_test.cpp
index 68fde3bf366..7207eb5002f 100644
--- a/src/mongo/db/repl/storage_interface_impl_test.cpp
+++ b/src/mongo/db/repl/storage_interface_impl_test.cpp
@@ -1530,6 +1530,51 @@ TEST_F(StorageInterfaceImplTest, FindByIdReturnsDocumentWhenDocumentExists) {
     ASSERT_BSONOBJ_EQ(doc2, unittest::assertGet(storage.findById(opCtx, nss, doc2["_id"])));
 }
 
+TEST_F(StorageInterfaceImplTest, DeleteByIdReturnsNamespaceNotFoundWhenDatabaseDoesNotExist) {
+    auto opCtx = getOperationContext();
+    StorageInterfaceImpl storage;
+    NamespaceString nss("nosuchdb.coll");
+    auto doc = BSON("_id" << 0 << "x" << 0);
+    ASSERT_EQUALS(ErrorCodes::NamespaceNotFound,
+                  storage.deleteById(opCtx, nss, doc["_id"]).getStatus());
+}
+
+TEST_F(StorageInterfaceImplTest, DeleteByIdReturnsNoSuchKeyWhenCollectionIsEmpty) {
+    auto opCtx = getOperationContext();
+    StorageInterfaceImpl storage;
+    auto nss = makeNamespace(_agent);
+    ASSERT_OK(storage.createCollection(opCtx, nss, CollectionOptions()));
+    auto doc = BSON("_id" << 0 << "x" << 0);
+    ASSERT_EQUALS(ErrorCodes::NoSuchKey, storage.deleteById(opCtx, nss, doc["_id"]).getStatus());
+    _assertDocumentsInCollectionEquals(opCtx, nss, {});
+}
+
+TEST_F(StorageInterfaceImplTest, DeleteByIdReturnsNoSuchKeyWhenDocumentIsNotFound) {
+    auto opCtx = getOperationContext();
+    StorageInterfaceImpl storage;
+    auto nss = makeNamespace(_agent);
+    ASSERT_OK(storage.createCollection(opCtx, nss, CollectionOptions()));
+    auto doc1 = BSON("_id" << 0 << "x" << 0);
+    auto doc2 = BSON("_id" << 1 << "x" << 1);
+    auto doc3 = BSON("_id" << 2 << "x" << 2);
+    ASSERT_OK(storage.insertDocuments(opCtx, nss, {doc1, doc3}));
+    ASSERT_EQUALS(ErrorCodes::NoSuchKey, storage.deleteById(opCtx, nss, doc2["_id"]).getStatus());
+    _assertDocumentsInCollectionEquals(opCtx, nss, {doc1, doc3});
+}
+
+TEST_F(StorageInterfaceImplTest, DeleteByIdReturnsDocumentWhenDocumentExists) {
+    auto opCtx = getOperationContext();
+    StorageInterfaceImpl storage;
+    auto nss = makeNamespace(_agent);
+    ASSERT_OK(storage.createCollection(opCtx, nss, CollectionOptions()));
+    auto doc1 = BSON("_id" << 0 << "x" << 0);
+    auto doc2 = BSON("_id" << 1 << "x" << 1);
+    auto doc3 = BSON("_id" << 2 << "x" << 2);
+    ASSERT_OK(storage.insertDocuments(opCtx, nss, {doc1, doc2, doc3}));
+    ASSERT_BSONOBJ_EQ(doc2, unittest::assertGet(storage.deleteById(opCtx, nss, doc2["_id"])));
+    _assertDocumentsInCollectionEquals(opCtx, nss, {doc1, doc3});
+}
+
 TEST_F(StorageInterfaceImplTest,
        UpsertSingleDocumentReturnsNamespaceNotFoundWhenDatabaseDoesNotExist) {
     auto opCtx = getOperationContext();
diff --git a/src/mongo/db/repl/storage_interface_mock.h b/src/mongo/db/repl/storage_interface_mock.h
index f6df26cd2df..95c31e6c5e5 100644
--- a/src/mongo/db/repl/storage_interface_mock.h
+++ b/src/mongo/db/repl/storage_interface_mock.h
@@ -205,6 +205,12 @@ public:
         return Status{ErrorCodes::IllegalOperation, "findById not implemented."};
     }
 
+    StatusWith<BSONObj> deleteById(OperationContext* opCtx,
+                                   const NamespaceString& nss,
+                                   const BSONElement& idKey) override {
+        return Status{ErrorCodes::IllegalOperation, "deleteById not implemented."};
+    }
+
     Status upsertById(OperationContext* opCtx,
                       const NamespaceString& nss,
                       const BSONElement& idKey,