SERVER-33725 use OplogEntry to parse opType in SyncTail::syncApply

4 files changed, 28 insertions, 35 deletions

diff --git a/src/mongo/db/repl/oplog_entry.cpp b/src/mongo/db/repl/oplog_entry.cpp
index 45fafaf06e8..722ed0e596c 100644
--- a/src/mongo/db/repl/oplog_entry.cpp
+++ b/src/mongo/db/repl/oplog_entry.cpp
@@ -227,8 +227,9 @@ bool OplogEntry::isCommand() const {
     return getOpType() == OpTypeEnum::kCommand;
 }
 
-bool OplogEntry::isCrudOpType() const {
-    switch (getOpType()) {
+// static
+bool OplogEntry::isCrudOpType(OpTypeEnum opType) {
+    switch (opType) {
         case OpTypeEnum::kInsert:
         case OpTypeEnum::kDelete:
         case OpTypeEnum::kUpdate:
@@ -240,6 +241,10 @@ bool OplogEntry::isCrudOpType() const {
     MONGO_UNREACHABLE;
 }
 
+bool OplogEntry::isCrudOpType() const {
+    return isCrudOpType(getOpType());
+}
+
 BSONElement OplogEntry::getIdElement() const {
     invariant(isCrudOpType());
     if (getOpType() == OpTypeEnum::kUpdate) {
diff --git a/src/mongo/db/repl/oplog_entry.h b/src/mongo/db/repl/oplog_entry.h
index e6eb921d985..6325891d6d7 100644
--- a/src/mongo/db/repl/oplog_entry.h
+++ b/src/mongo/db/repl/oplog_entry.h
@@ -107,6 +107,7 @@ public:
     /**
      * Returns if the oplog entry is for a CRUD operation.
      */
+    static bool isCrudOpType(OpTypeEnum opType);
     bool isCrudOpType() const;
 
     /**
diff --git a/src/mongo/db/repl/sync_tail.cpp b/src/mongo/db/repl/sync_tail.cpp
index 1a5c155d89d..d91e02f7c40 100644
--- a/src/mongo/db/repl/sync_tail.cpp
+++ b/src/mongo/db/repl/sync_tail.cpp
@@ -147,16 +147,6 @@ ServerStatusMetricField<Counter64> displayAttemptsToBecomeSecondary(
 TimerStats applyBatchStats;
 ServerStatusMetricField<TimerStats> displayOpBatchesApplied("repl.apply.batches", &applyBatchStats);
 
-bool isCrudOpType(const char* field) {
-    switch (field[0]) {
-        case 'd':
-        case 'i':
-        case 'u':
-            return field[1] == 0;
-    }
-    return false;
-}
-
 class ApplyBatchFinalizer {
 public:
     ApplyBatchFinalizer(ReplicationCoordinator* replCoord) : _replCoord(replCoord) {}
@@ -331,8 +321,6 @@ Status SyncTail::syncApply(OperationContext* opCtx,
 
     const NamespaceString nss(op.getStringField("ns"));
 
-    const char* opType = op["op"].valuestrsafe();
-
     auto applyOp = [&](Database* db) {
         // For non-initial-sync, we convert updates to upserts
         // to suppress errors when replaying oplog entries.
@@ -356,16 +344,20 @@ Status SyncTail::syncApply(OperationContext* opCtx,
         return status;
     };
 
-    bool isNoOp = opType[0] == 'n';
-    if (isNoOp || (opType[0] == 'i' && nss.isSystemDotIndexes())) {
-        if (isNoOp && nss.db() == "")
+    auto opType = OpType_parse(IDLParserErrorContext("syncApply"), op["op"].valuestrsafe());
+
+    if (opType == OpTypeEnum::kNoop) {
+        if (nss.db() == "") {
             return Status::OK();
+        }
         Lock::DBLock dbLock(opCtx, nss.db(), MODE_X);
         OldClientContext ctx(opCtx, nss.ns());
         return applyOp(ctx.db());
-    }
-
-    if (isCrudOpType(opType)) {
+    } else if (opType == OpTypeEnum::kInsert && nss.isSystemDotIndexes()) {
+        Lock::DBLock dbLock(opCtx, nss.db(), MODE_X);
+        OldClientContext ctx(opCtx, nss.ns());
+        return applyOp(ctx.db());
+    } else if (OplogEntry::isCrudOpType(opType)) {
         return writeConflictRetry(opCtx, "syncApply_CRUD", nss.ns(), [&] {
             // Need to throw instead of returning a status for it to be properly ignored.
             try {
@@ -379,16 +371,14 @@ Status SyncTail::syncApply(OperationContext* opCtx,
             } catch (ExceptionFor<ErrorCodes::NamespaceNotFound>& ex) {
                 // Delete operations on non-existent namespaces can be treated as successful for
                 // idempotency reasons.
-                if (opType[0] == 'd') {
+                if (opType == OpTypeEnum::kDelete) {
                     return Status::OK();
                 }
                 ex.addContext(str::stream() << "Failed to apply operation: " << redact(op));
                 throw;
             }
         });
-    }
-
-    if (opType[0] == 'c') {
+    } else if (opType == OpTypeEnum::kCommand) {
         return writeConflictRetry(opCtx, "syncApply_command", nss.ns(), [&] {
             // a command may need a global write lock. so we will conservatively go
             // ahead and grab one here. suboptimal. :-(
@@ -401,11 +391,7 @@ Status SyncTail::syncApply(OperationContext* opCtx,
         });
     }
 
-    // unknown opType
-    str::stream ss;
-    ss << "bad opType '" << opType << "' in oplog entry: " << redact(op);
-    error() << std::string(ss);
-    return Status(ErrorCodes::BadValue, ss);
+    MONGO_UNREACHABLE;
 }
 
 Status SyncTail::syncApply(OperationContext* opCtx,
diff --git a/src/mongo/db/repl/sync_tail_test.cpp b/src/mongo/db/repl/sync_tail_test.cpp
index aac8927f9f9..96b77faa606 100644
--- a/src/mongo/db/repl/sync_tail_test.cpp
+++ b/src/mongo/db/repl/sync_tail_test.cpp
@@ -199,10 +199,11 @@ auto parseFromOplogEntryArray(const BSONObj& obj, int elem) {
 TEST_F(SyncTailTest, SyncApplyNoNamespaceBadOp) {
     const BSONObj op = BSON("op"
                             << "x");
-    ASSERT_EQUALS(
-        ErrorCodes::BadValue,
+    ASSERT_THROWS(
         SyncTail::syncApply(
-            _opCtx.get(), op, OplogApplication::Mode::kInitialSync, _applyOp, _applyCmd, _incOps));
+            _opCtx.get(), op, OplogApplication::Mode::kInitialSync, _applyOp, _applyCmd, _incOps)
+            .ignore(),
+        ExceptionFor<ErrorCodes::BadValue>);
     ASSERT_EQUALS(0U, _opsApplied);
 }
 
@@ -219,11 +220,11 @@ TEST_F(SyncTailTest, SyncApplyBadOp) {
                             << "x"
                             << "ns"
                             << "test.t");
-    ASSERT_EQUALS(
-        ErrorCodes::BadValue,
+    ASSERT_THROWS(
         SyncTail::syncApply(
             _opCtx.get(), op, OplogApplication::Mode::kInitialSync, _applyOp, _applyCmd, _incOps)
-            .code());
+            .ignore(),
+        ExceptionFor<ErrorCodes::BadValue>);
     ASSERT_EQUALS(0U, _opsApplied);
 }