SERVER-45105 Keep allowing scope and finalize to be explicit null in mapReduce command

5 files changed, 96 insertions, 37 deletions

diff --git a/src/mongo/db/commands/map_reduce.idl b/src/mongo/db/commands/map_reduce.idl
index 8c16042bf45..6c89d81d34a 100644
--- a/src/mongo/db/commands/map_reduce.idl
+++ b/src/mongo/db/commands/map_reduce.idl
@@ -52,6 +52,13 @@ types:
         serializer: mongo::MapReduceJavascriptCode::serializeToBSON
         deserializer: mongo::MapReduceJavascriptCode::parseFromBSON
 
+    mapReduceJavascriptCodeNullPermittedType:
+        bson_serialization_type: any
+        description: "Holds Javascript code passed as command input, but also accepts null"
+        cpp_type: "mongo::MapReduceJavascriptCodeOrNull"
+        serializer: mongo::MapReduceJavascriptCodeOrNull::serializeToBSON
+        deserializer: mongo::MapReduceJavascriptCodeOrNull::parseFromBSON
+
     mapReduceGlobalVariableScopeType:
         bson_serialization_type: any
         description: "Holds a mapping of Javascript global variables"
@@ -67,11 +74,11 @@ commands:
         strict: true
         fields:
             map:
-                description: "Javascript code to run as the map operation which associates a value 
+                description: "Javascript code to run as the map operation which associates a value
                     with a key and emits the key and value pair."
                 type: mapReduceJavascriptCodeType
             reduce:
-                description: "Javascript code to run as the map operation which reduces all the 
+                description: "Javascript code to run as the map operation which reduces all the
                     values associated with a particular key to a single value."
                 type: mapReduceJavascriptCodeType
             out:
@@ -79,7 +86,7 @@ commands:
                 type: mapReduceOutOptionsType
                 cpp_name: outOptions
             query:
-                description: "Query object in match language to use as a filter applied before the 
+                description: "Query object in match language to use as a filter applied before the
                     map step."
                 type: object
                 optional: true
@@ -97,19 +104,19 @@ commands:
                 optional: true
             finalize:
                 description: "Javascript code to run after the reduce operation."
-                type:  mapReduceJavascriptCodeType
+                type:  mapReduceJavascriptCodeNullPermittedType
                 optional: true
             scope:
                 description: "Javascript global variable mapping for map, reduce and finalize."
                 type:  mapReduceGlobalVariableScopeType
                 optional: true
             verbose:
-                description: "Specifies whether to include the timing information in the result 
+                description: "Specifies whether to include the timing information in the result
                     information."
                 type: bool
                 optional: true
             bypassDocumentValidation:
-                description: "Causes the out portion of the operation to ignore the output 
+                description: "Causes the out portion of the operation to ignore the output
                     collection's document validation."
                 type: bool
                 optional: true
diff --git a/src/mongo/db/commands/map_reduce_agg_test.cpp b/src/mongo/db/commands/map_reduce_agg_test.cpp
index 7023853ac95..9845fa05ff0 100644
--- a/src/mongo/db/commands/map_reduce_agg_test.cpp
+++ b/src/mongo/db/commands/map_reduce_agg_test.cpp
@@ -158,7 +158,8 @@ TEST(MapReduceAggTest, testFeatureLadenTranslate) {
     mr.setSort(BSON("foo" << 1));
     mr.setQuery(BSON("foo"
                      << "fooval"));
-    mr.setFinalize(boost::make_optional(MapReduceJavascriptCode{finalizeJavascript.toString()}));
+    mr.setFinalize(
+        boost::make_optional(MapReduceJavascriptCodeOrNull{finalizeJavascript.toString()}));
     boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest(nss));
     auto pipeline = map_reduce_common::translateFromMR(mr, expCtx);
     auto& sources = pipeline->getSources();
diff --git a/src/mongo/db/commands/map_reduce_global_variable_scope.h b/src/mongo/db/commands/map_reduce_global_variable_scope.h
index 31e7de551a1..7207d30b532 100644
--- a/src/mongo/db/commands/map_reduce_global_variable_scope.h
+++ b/src/mongo/db/commands/map_reduce_global_variable_scope.h
@@ -29,6 +29,8 @@
 
 #pragma once
 
+#include <boost/optional.hpp>
+
 #include "mongo/base/string_data.h"
 #include "mongo/bson/bsonelement.h"
 #include "mongo/bson/bsonobj.h"
@@ -41,25 +43,31 @@ namespace mongo {
 class MapReduceGlobalVariableScope {
 public:
     static MapReduceGlobalVariableScope parseFromBSON(const BSONElement& element) {
+        if (element.type() == jstNULL) {
+            return MapReduceGlobalVariableScope();
+        }
         uassert(ErrorCodes::BadValue, "'scope' must be an object", element.type() == Object);
         return MapReduceGlobalVariableScope(element.embeddedObject());
     }
 
     MapReduceGlobalVariableScope() = default;
-    MapReduceGlobalVariableScope(const BSONObj& obj) : obj(obj.getOwned()) {}
+    MapReduceGlobalVariableScope(const BSONObj& obj) : obj(boost::make_optional(obj.getOwned())) {}
 
     void serializeToBSON(StringData fieldName, BSONObjBuilder* builder) const {
-        builder->append(fieldName, obj);
+        if (obj == boost::none) {
+            builder->append(fieldName, "null");
+        }
+        builder->append(fieldName, obj.get());
     }
 
-    BSONObj getObj() const {
+    boost::optional<BSONObj> getObj() const {
         return obj;
     }
 
 private:
     // Initializers for global variables. These will be directly executed as Javascript. This is
     // left as a BSONObj for that API.
-    BSONObj obj;
+    boost::optional<BSONObj> obj;
 };
 
 }  // namespace mongo
diff --git a/src/mongo/db/commands/map_reduce_javascript_code.h b/src/mongo/db/commands/map_reduce_javascript_code.h
index fbe4cbae203..f51868c89b8 100644
--- a/src/mongo/db/commands/map_reduce_javascript_code.h
+++ b/src/mongo/db/commands/map_reduce_javascript_code.h
@@ -29,6 +29,7 @@
 
 #pragma once
 
+#include <boost/optional.hpp>
 #include <string>
 
 #include "mongo/base/string_data.h"
@@ -65,4 +66,44 @@ private:
     std::string code;
 };
 
+/**
+ * Same as above, but allows for null. This is required for older versions of the Java driver which
+ * send finalize: null if the argument is omitted by the user.
+ */
+class MapReduceJavascriptCodeOrNull {
+public:
+    static MapReduceJavascriptCodeOrNull parseFromBSON(const BSONElement& element) {
+        if (element.type() == jstNULL) {
+            return MapReduceJavascriptCodeOrNull(boost::none);
+        }
+        uassert(ErrorCodes::BadValue,
+                str::stream() << "'" << element.fieldNameStringData()
+                              << "' must be of string or code type",
+                element.type() == String || element.type() == Code);
+        return MapReduceJavascriptCodeOrNull(boost::make_optional(element._asCode()));
+    }
+
+    MapReduceJavascriptCodeOrNull() = default;
+    MapReduceJavascriptCodeOrNull(boost::optional<std::string> code) : code(code) {}
+
+    void serializeToBSON(StringData fieldName, BSONObjBuilder* builder) const {
+        if (code == boost::none) {
+            (*builder) << fieldName << "null";
+        } else {
+            (*builder) << fieldName << code.get();
+        }
+    }
+
+    auto getCode() const {
+        return code;
+    }
+
+    bool hasCode() const {
+        return code != boost::none;
+    }
+
+private:
+    boost::optional<std::string> code;
+};
+
 }  // namespace mongo
diff --git a/src/mongo/db/commands/mr_common.cpp b/src/mongo/db/commands/mr_common.cpp
index 231b68173ba..c6b8750adbf 100644
--- a/src/mongo/db/commands/mr_common.cpp
+++ b/src/mongo/db/commands/mr_common.cpp
@@ -136,26 +136,29 @@ auto translateReduce(boost::intrusive_ptr<ExpressionContext> expCtx, std::string
                                        boost::none);
 }
 
-auto translateFinalize(boost::intrusive_ptr<ExpressionContext> expCtx, std::string code) {
-    auto jsExpression = ExpressionInternalJs::create(
-        expCtx,
-        ExpressionArray::create(
+auto translateFinalize(boost::intrusive_ptr<ExpressionContext> expCtx,
+                       MapReduceJavascriptCodeOrNull codeObj) {
+    return codeObj.getCode().map([&](auto&& code) {
+        auto jsExpression = ExpressionInternalJs::create(
             expCtx,
-            make_vector<boost::intrusive_ptr<Expression>>(
-                ExpressionFieldPath::parse(expCtx, "$_id", expCtx->variablesParseState),
-                ExpressionFieldPath::parse(expCtx, "$value", expCtx->variablesParseState))),
-        code);
-    auto node = std::make_unique<projection_executor::InclusionNode>(
-        ProjectionPolicies{ProjectionPolicies::DefaultIdPolicy::kIncludeId});
-    node->addProjectionForPath(FieldPath{"_id"s});
-    node->addExpressionForPath(FieldPath{"value"s}, std::move(jsExpression));
-    auto inclusion = std::unique_ptr<TransformerInterface>{
-        std::make_unique<projection_executor::InclusionProjectionExecutor>(
-            expCtx,
-            ProjectionPolicies{ProjectionPolicies::DefaultIdPolicy::kIncludeId},
-            std::move(node))};
-    return make_intrusive<DocumentSourceSingleDocumentTransformation>(
-        expCtx, std::move(inclusion), DocumentSourceProject::kStageName, false);
+            ExpressionArray::create(
+                expCtx,
+                make_vector<boost::intrusive_ptr<Expression>>(
+                    ExpressionFieldPath::parse(expCtx, "$_id", expCtx->variablesParseState),
+                    ExpressionFieldPath::parse(expCtx, "$value", expCtx->variablesParseState))),
+            code);
+        auto node = std::make_unique<projection_executor::InclusionNode>(
+            ProjectionPolicies{ProjectionPolicies::DefaultIdPolicy::kIncludeId});
+        node->addProjectionForPath(FieldPath{"_id"s});
+        node->addExpressionForPath(FieldPath{"value"s}, std::move(jsExpression));
+        auto inclusion = std::unique_ptr<TransformerInterface>{
+            std::make_unique<projection_executor::InclusionProjectionExecutor>(
+                expCtx,
+                ProjectionPolicies{ProjectionPolicies::DefaultIdPolicy::kIncludeId},
+                std::move(node))};
+        return make_intrusive<DocumentSourceSingleDocumentTransformation>(
+            expCtx, std::move(inclusion), DocumentSourceProject::kStageName, false);
+    });
 }
 
 auto translateOutReplace(boost::intrusive_ptr<ExpressionContext> expCtx,
@@ -177,7 +180,7 @@ auto translateOutMerge(boost::intrusive_ptr<ExpressionContext> expCtx, Namespace
 auto translateOutReduce(boost::intrusive_ptr<ExpressionContext> expCtx,
                         NamespaceString targetNss,
                         std::string reduceCode,
-                        boost::optional<MapReduceJavascriptCode> finalizeCode) {
+                        boost::optional<MapReduceJavascriptCodeOrNull> finalizeCode) {
     // Because of communication for sharding, $merge must hold on to a serializable BSON object
     // at the moment so we reparse here. Note that the reduce function signature expects 2
     // arguments, the first being the key and the second being the array of values to reduce.
@@ -191,10 +194,10 @@ auto translateOutReduce(boost::intrusive_ptr<ExpressionContext> expCtx,
     auto pipelineSpec = boost::make_optional(std::vector<BSONObj>{reduceSpec});
 
     // Build finalize $project stage if given.
-    if (finalizeCode) {
+    if (finalizeCode && finalizeCode->hasCode()) {
         auto finalizeObj = BSON("args" << BSON_ARRAY("$_id"
                                                      << "$value")
-                                       << "eval" << finalizeCode->getCode());
+                                       << "eval" << finalizeCode->getCode().get());
         auto finalizeSpec =
             BSON(DocumentSourceProject::kStageName
                  << BSON("value" << BSON(ExpressionInternalJs::kExpressionName << finalizeObj)));
@@ -215,7 +218,7 @@ auto translateOut(boost::intrusive_ptr<ExpressionContext> expCtx,
                   const OutputType outputType,
                   NamespaceString targetNss,
                   std::string reduceCode,
-                  boost::optional<MapReduceJavascriptCode> finalizeCode) {
+                  boost::optional<MapReduceJavascriptCodeOrNull> finalizeCode) {
     switch (outputType) {
         case OutputType::Replace:
             return boost::make_optional(translateOutReplace(expCtx, targetNss));
@@ -384,9 +387,8 @@ std::unique_ptr<Pipeline, PipelineDeleter> translateFromMR(
                 translateMap(expCtx, parsedMr.getMap().getCode()),
                 DocumentSourceUnwind::create(expCtx, "emits", false, boost::none),
                 translateReduce(expCtx, parsedMr.getReduce().getCode()),
-                parsedMr.getFinalize().map([&](auto&& finalize) {
-                    return translateFinalize(expCtx, parsedMr.getFinalize()->getCode());
-                }),
+                parsedMr.getFinalize().flat_map(
+                    [&](auto&& finalize) { return translateFinalize(expCtx, finalize); }),
                 translateOut(expCtx,
                              outType,
                              std::move(outNss),