SERVER-74155 Make working with array<variant<BulkWriteInsertOp, BulkWriteUpdateOp, BulkWriteDeleteOp>> easier

7 files changed, 187 insertions, 86 deletions

diff --git a/src/mongo/db/commands/SConscript b/src/mongo/db/commands/SConscript
index 7e59024159b..b3e9df206ad 100644
--- a/src/mongo/db/commands/SConscript
+++ b/src/mongo/db/commands/SConscript
@@ -270,7 +270,7 @@ env.Library(
 
 env.Library(
     target='bulk_write_parser',
-    source=['bulk_write.idl', 'bulk_write_parser.cpp'],
+    source=['bulk_write.idl', 'bulk_write_parser.cpp', 'bulk_write_crud_op.cpp'],
     LIBDEPS_PRIVATE=[
         '$BUILD_DIR/mongo/crypto/fle_fields',
         '$BUILD_DIR/mongo/db/commands',
diff --git a/src/mongo/db/commands/bulk_write.cpp b/src/mongo/db/commands/bulk_write.cpp
index d53a1a02215..6d6dd260afd 100644
--- a/src/mongo/db/commands/bulk_write.cpp
+++ b/src/mongo/db/commands/bulk_write.cpp
@@ -38,6 +38,7 @@
 #include "mongo/db/catalog/collection_operation_source.h"
 #include "mongo/db/catalog/document_validation.h"
 #include "mongo/db/commands.h"
+#include "mongo/db/commands/bulk_write_crud_op.h"
 #include "mongo/db/commands/bulk_write_gen.h"
 #include "mongo/db/commands/bulk_write_parser.h"
 #include "mongo/db/cursor_manager.h"
@@ -212,20 +213,6 @@ private:
     std::vector<BulkWriteReplyItem> _replies;
 };
 
-enum OperationType { kInsert = 1, kUpdate = 2, kDelete = 3 };
-
-OperationType getOpType(const stdx::variant<mongo::BulkWriteInsertOp,
-                                            mongo::BulkWriteUpdateOp,
-                                            mongo::BulkWriteDeleteOp>& op) {
-    return stdx::visit(
-        OverloadedVisitor{
-            [](const mongo::BulkWriteInsertOp& value) { return OperationType::kInsert; },
-            [](const mongo::BulkWriteUpdateOp& value) { return OperationType::kUpdate; },
-            [](const mongo::BulkWriteDeleteOp& value) { return OperationType::kDelete; },
-        },
-        op);
-}
-
 int32_t getStatementId(OperationContext* opCtx,
                        const BulkWriteCommandRequest& req,
                        const size_t currentOpIdx) {
@@ -245,25 +232,25 @@ int32_t getStatementId(OperationContext* opCtx,
 }
 
 bool handleInsertOp(OperationContext* opCtx,
+                    const BulkWriteInsertOp* op,
                     const BulkWriteCommandRequest& req,
                     size_t currentOpIdx,
                     InsertBatch& batch) {
     const auto& nsInfo = req.getNsInfo();
-    // Caller needs to check the type with getOpType or this stdx::get<>() will throw.
-    auto& op = stdx::get<mongo::BulkWriteInsertOp>(req.getOps()[currentOpIdx]);
-    auto idx = op.getInsert();
+    auto idx = op->getInsert();
 
     auto stmtId = getStatementId(opCtx, req, currentOpIdx);
     bool containsDotsAndDollarsField = false;
-    auto fixedDoc = fixDocumentForInsert(opCtx, op.getDocument(), &containsDotsAndDollarsField);
+    auto fixedDoc = fixDocumentForInsert(opCtx, op->getDocument(), &containsDotsAndDollarsField);
     BSONObj toInsert =
-        fixedDoc.getValue().isEmpty() ? op.getDocument() : std::move(fixedDoc.getValue());
+        fixedDoc.getValue().isEmpty() ? op->getDocument() : std::move(fixedDoc.getValue());
 
     // TODO handle !fixedDoc.isOk() condition like in write_ops_exec::performInserts.
     return batch.addToBatch(opCtx, currentOpIdx, stmtId, nsInfo[idx], toInsert);
 }
 
 bool handleUpdateOp(OperationContext* opCtx,
+                    const BulkWriteUpdateOp* op,
                     const BulkWriteCommandRequest& req,
                     size_t currentOpIdx,
                     std::function<void(OperationContext*, int, const SingleWriteResult&)> replyCB) {
@@ -274,6 +261,7 @@ bool handleUpdateOp(OperationContext* opCtx,
 }
 
 bool handleDeleteOp(OperationContext* opCtx,
+                    const BulkWriteDeleteOp* op,
                     const BulkWriteCommandRequest& req,
                     size_t currentOpIdx,
                     std::function<void(OperationContext*, int, const SingleWriteResult&)> replyCB) {
@@ -315,19 +303,20 @@ std::vector<BulkWriteReplyItem> performWrites(OperationContext* opCtx,
     size_t idx = 0;
 
     for (; idx < ops.size(); ++idx) {
-        auto opType = getOpType(ops[idx]);
+        auto op = BulkWriteCRUDOp(ops[idx]);
+        auto opType = op.getType();
 
-        if (opType == kInsert) {
-            if (!handleInsertOp(opCtx, req, idx, batch)) {
+        if (opType == BulkWriteCRUDOp::kInsert) {
+            if (!handleInsertOp(opCtx, op.getInsert(), req, idx, batch)) {
                 // Insert write failed can no longer continue.
                 break;
             }
-        } else if (opType == kUpdate) {
+        } else if (opType == BulkWriteCRUDOp::kUpdate) {
             // Flush insert ops before handling update ops.
             if (!batch.flush(opCtx)) {
                 break;
             }
-            if (!handleUpdateOp(opCtx, req, idx, updateCB)) {
+            if (!handleUpdateOp(opCtx, op.getUpdate(), req, idx, updateCB)) {
                 // Update write failed can no longer continue.
                 break;
             }
@@ -336,7 +325,7 @@ std::vector<BulkWriteReplyItem> performWrites(OperationContext* opCtx,
             if (!batch.flush(opCtx)) {
                 break;
             }
-            if (!handleDeleteOp(opCtx, req, idx, deleteCB)) {
+            if (!handleDeleteOp(opCtx, op.getDelete(), req, idx, deleteCB)) {
                 // Delete write failed can no longer continue.
                 break;
             }
@@ -421,19 +410,11 @@ public:
             const auto& nsInfo = req.getNsInfo();
 
             for (const auto& op : ops) {
-                // TODO(SERVER-74155) revisit logging value.toBSON() instead of 'op' once we have a
-                // helper. Issue is that we want to avoid serialization in the happy path so the
-                // visit below is not the correct place to do this right now.
-                unsigned int nsInfoIdx = stdx::visit(
-                    OverloadedVisitor{
-                        [](const mongo::BulkWriteInsertOp& value) { return value.getInsert(); },
-                        [](const mongo::BulkWriteUpdateOp& value) { return value.getUpdate(); },
-                        [](const mongo::BulkWriteDeleteOp& value) {
-                            return value.getDeleteCommand();
-                        }},
-                    op);
+                const auto& bulkWriteOp = BulkWriteCRUDOp(op);
+                unsigned int nsInfoIdx = bulkWriteOp.getNsInfoIdx();
                 uassert(ErrorCodes::BadValue,
-                        str::stream() << "BulkWrite ops entry has an invalid nsInfo index.",
+                        str::stream() << "BulkWrite ops entry " << bulkWriteOp.toBSON()
+                                      << " has an invalid nsInfo index.",
                         nsInfoIdx < nsInfo.size());
             }
 
@@ -475,29 +456,12 @@ public:
 
             // Iterate over each op and assign the appropriate actions to the namespace privilege.
             for (const auto& op : ops) {
-                // TODO(SERVER-74155) revisit logging value.toBSON() instead of 'op' once we have a
-                // helper. Issue is that we want to avoid serialization in the happy path so the
-                // visit below is not the correct place to do this right now.
-                ActionSet newActions;
-                unsigned int nsInfoIdx = stdx::visit(
-                    OverloadedVisitor{[&newActions](const mongo::BulkWriteInsertOp& value) {
-                                          newActions.addAction(ActionType::insert);
-                                          return value.getInsert();
-                                      },
-                                      [&newActions](const mongo::BulkWriteUpdateOp& value) {
-                                          if (value.getUpsert()) {
-                                              newActions.addAction(ActionType::insert);
-                                          }
-                                          newActions.addAction(ActionType::update);
-                                          return value.getUpdate();
-                                      },
-                                      [&newActions](const mongo::BulkWriteDeleteOp& value) {
-                                          newActions.addAction(ActionType::remove);
-                                          return value.getDeleteCommand();
-                                      }},
-                    op);
+                const auto& bulkWriteOp = BulkWriteCRUDOp(op);
+                ActionSet newActions = bulkWriteOp.getActions();
+                unsigned int nsInfoIdx = bulkWriteOp.getNsInfoIdx();
                 uassert(ErrorCodes::BadValue,
-                        str::stream() << "BulkWrite ops entry has an invalid nsInfo index.",
+                        str::stream() << "BulkWrite ops entry " << bulkWriteOp.toBSON()
+                                      << " has an invalid nsInfo index.",
                         nsInfoIdx < nsInfo.size());
 
                 auto& privilege = privileges[nsInfoIdx];
diff --git a/src/mongo/db/commands/bulk_write_crud_op.cpp b/src/mongo/db/commands/bulk_write_crud_op.cpp
new file mode 100644
index 00000000000..68aeb5a6d5e
--- /dev/null
+++ b/src/mongo/db/commands/bulk_write_crud_op.cpp
@@ -0,0 +1,94 @@
+/**
+ *    Copyright (C) 2023-present MongoDB, Inc.
+ *
+ *    This program is free software: you can redistribute it and/or modify
+ *    it under the terms of the Server Side Public License, version 1,
+ *    as published by MongoDB, Inc.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    Server Side Public License for more details.
+ *
+ *    You should have received a copy of the Server Side Public License
+ *    along with this program. If not, see
+ *    <http://www.mongodb.com/licensing/server-side-public-license>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the Server Side Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#include "mongo/db/commands/bulk_write_crud_op.h"
+
+namespace mongo {
+
+BulkWriteCRUDOp::BulkWriteCRUDOp(const stdx::variant<mongo::BulkWriteInsertOp,
+                                                     mongo::BulkWriteUpdateOp,
+                                                     mongo::BulkWriteDeleteOp>& op)
+    : _op{op}, _type{op.index()} {}
+
+BulkWriteCRUDOp::OpType BulkWriteCRUDOp::getType() const {
+    return _type;
+}
+
+unsigned int BulkWriteCRUDOp::getNsInfoIdx() const {
+    return stdx::visit(
+        OverloadedVisitor{[](const mongo::BulkWriteInsertOp& value) { return value.getInsert(); },
+                          [](const mongo::BulkWriteUpdateOp& value) { return value.getUpdate(); },
+                          [](const mongo::BulkWriteDeleteOp& value) {
+                              return value.getDeleteCommand();
+                          }},
+        _op);
+}
+
+ActionSet BulkWriteCRUDOp::getActions() const {
+    ActionSet newActions;
+    stdx::visit(OverloadedVisitor{[&newActions](const mongo::BulkWriteInsertOp& value) {
+                                      newActions.addAction(ActionType::insert);
+                                  },
+                                  [&newActions](const mongo::BulkWriteUpdateOp& value) {
+                                      if (value.getUpsert()) {
+                                          newActions.addAction(ActionType::insert);
+                                      }
+                                      newActions.addAction(ActionType::update);
+                                  },
+                                  [&newActions](const mongo::BulkWriteDeleteOp& value) {
+                                      newActions.addAction(ActionType::remove);
+                                  }},
+                _op);
+
+    return newActions;
+}
+
+const mongo::BulkWriteInsertOp* BulkWriteCRUDOp::getInsert() const {
+    return stdx::get_if<BulkWriteCRUDOp::OpType::kInsert>(&_op);
+}
+
+const mongo::BulkWriteUpdateOp* BulkWriteCRUDOp::getUpdate() const {
+    return stdx::get_if<BulkWriteCRUDOp::OpType::kUpdate>(&_op);
+}
+
+const mongo::BulkWriteDeleteOp* BulkWriteCRUDOp::getDelete() const {
+    return stdx::get_if<BulkWriteCRUDOp::OpType::kDelete>(&_op);
+}
+
+mongo::BSONObj BulkWriteCRUDOp::toBSON() const {
+    return stdx::visit(
+        OverloadedVisitor{[](const mongo::BulkWriteInsertOp& value) { return value.toBSON(); },
+                          [](const mongo::BulkWriteUpdateOp& value) { return value.toBSON(); },
+                          [](const mongo::BulkWriteDeleteOp& value) {
+                              return value.toBSON();
+                          }},
+        _op);
+}
+
+}  // namespace mongo
diff --git a/src/mongo/db/commands/bulk_write_crud_op.h b/src/mongo/db/commands/bulk_write_crud_op.h
new file mode 100644
index 00000000000..c2863b4cb61
--- /dev/null
+++ b/src/mongo/db/commands/bulk_write_crud_op.h
@@ -0,0 +1,65 @@
+/**
+ *    Copyright (C) 2023-present MongoDB, Inc.
+ *
+ *    This program is free software: you can redistribute it and/or modify
+ *    it under the terms of the Server Side Public License, version 1,
+ *    as published by MongoDB, Inc.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    Server Side Public License for more details.
+ *
+ *    You should have received a copy of the Server Side Public License
+ *    along with this program. If not, see
+ *    <http://www.mongodb.com/licensing/server-side-public-license>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the Server Side Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#pragma once
+
+#include "mongo/db/auth/action_type.h"
+#include "mongo/db/commands/bulk_write_gen.h"
+#include "mongo/stdx/variant.h"
+
+namespace mongo {
+
+/**
+ * The BulkWriteCRUDOp class makes working with
+ * variant<BulkWriteInsertOp, BulkWriteUpdateOp, BulkWriteDeleteOp> easier.
+ */
+class BulkWriteCRUDOp {
+public:
+    enum OpType : size_t { kInsert = 0, kUpdate = 1, kDelete = 2 };
+
+    BulkWriteCRUDOp(const stdx::variant<mongo::BulkWriteInsertOp,
+                                        mongo::BulkWriteUpdateOp,
+                                        mongo::BulkWriteDeleteOp>& op);
+
+    OpType getType() const;
+    unsigned int getNsInfoIdx() const;
+    ActionSet getActions() const;
+    mongo::BSONObj toBSON() const;
+
+    const mongo::BulkWriteInsertOp* getInsert() const;
+    const mongo::BulkWriteUpdateOp* getUpdate() const;
+    const mongo::BulkWriteDeleteOp* getDelete() const;
+
+private:
+    const stdx::
+        variant<mongo::BulkWriteInsertOp, mongo::BulkWriteUpdateOp, mongo::BulkWriteDeleteOp>& _op;
+    OpType _type;
+};
+
+}  // namespace mongo
diff --git a/src/mongo/s/write_ops/batched_command_request.cpp b/src/mongo/s/write_ops/batched_command_request.cpp
index 20d634017f6..b0075f39765 100644
--- a/src/mongo/s/write_ops/batched_command_request.cpp
+++ b/src/mongo/s/write_ops/batched_command_request.cpp
@@ -71,14 +71,6 @@ BatchedCommandRequest constructBatchedCommandRequest(const OpMsgRequest& request
 
 }  // namespace
 
-// TODO(SERVER-74155): consider this for the helper class to access variant op.
-const mongo::BulkWriteInsertOp& getInsert(const stdx::variant<mongo::BulkWriteInsertOp,
-                                                              mongo::BulkWriteUpdateOp,
-                                                              mongo::BulkWriteDeleteOp>& op) {
-    // Caller needs to check the type with getOpType or this stdx::get<>() will throw.
-    return stdx::get<mongo::BulkWriteInsertOp>(op);
-}
-
 const boost::optional<LegacyRuntimeConstants> BatchedCommandRequest::kEmptyRuntimeConstants =
     boost::optional<LegacyRuntimeConstants>{};
 const boost::optional<BSONObj> BatchedCommandRequest::kEmptyLet = boost::optional<BSONObj>{};
diff --git a/src/mongo/s/write_ops/batched_command_request.h b/src/mongo/s/write_ops/batched_command_request.h
index e6a82d3de28..09c812cdc53 100644
--- a/src/mongo/s/write_ops/batched_command_request.h
+++ b/src/mongo/s/write_ops/batched_command_request.h
@@ -32,6 +32,7 @@
 #include <boost/optional.hpp>
 #include <memory>
 
+#include "mongo/db/commands/bulk_write_crud_op.h"
 #include "mongo/db/commands/bulk_write_gen.h"
 #include "mongo/db/ops/write_ops.h"
 #include "mongo/rpc/op_msg.h"
@@ -41,13 +42,6 @@
 
 namespace mongo {
 
-// TODO(SERVER-74155): consider this for the helper class to access variant op.
-// This function was introduced to mitigate an MSVC Internal compiler error around
-// stdx::get<T>(op).getDocument(), see SERVER-72092.
-const mongo::BulkWriteInsertOp& getInsert(const stdx::variant<mongo::BulkWriteInsertOp,
-                                                              mongo::BulkWriteUpdateOp,
-                                                              mongo::BulkWriteDeleteOp>& op);
-
 /**
  * This class wraps the different kinds of command requests into a generically usable write command
  * request that can be passed around.
@@ -271,7 +265,7 @@ public:
         } else {
             tassert(7263703, "invalid bulkWrite request reference", _bulkWriteRequest);
             const auto& op = _bulkWriteRequest->getOps()[_index];
-            return getInsert(op).getDocument();
+            return BulkWriteCRUDOp(op).getInsert()->getDocument();
         }
     }
     const auto& getUpdate() const {
diff --git a/src/mongo/s/write_ops/bulk_write_exec.cpp b/src/mongo/s/write_ops/bulk_write_exec.cpp
index 73e6d6fa0d3..5c606f06a38 100644
--- a/src/mongo/s/write_ops/bulk_write_exec.cpp
+++ b/src/mongo/s/write_ops/bulk_write_exec.cpp
@@ -177,16 +177,8 @@ StatusWith<bool> BulkWriteOp::target(const std::vector<std::unique_ptr<NSTargete
         // getTargeterFn:
         [&](const WriteOp& writeOp) -> const NSTargeter& {
             const auto opIdx = writeOp.getWriteItem().getItemIndex();
-            // TODO(SERVER-74155): Support bulkWrite insert, update and
-            // delete in a cleaner way.
-            const auto nsIdx = stdx::visit(
-                OverloadedVisitor{
-                    [](const mongo::BulkWriteInsertOp& value) { return value.getInsert(); },
-                    [](const mongo::BulkWriteUpdateOp& value) { return value.getUpdate(); },
-                    [](const mongo::BulkWriteDeleteOp& value) { return value.getDeleteCommand(); },
-                },
-                _clientRequest.getOps()[opIdx]);
-            return *targeters[nsIdx];
+            const auto& bulkWriteOp = BulkWriteCRUDOp(_clientRequest.getOps()[opIdx]);
+            return *targeters[bulkWriteOp.getNsInfoIdx()];
         },
         // getWriteSizeFn:
         [&](const WriteOp& writeOp) {