SERVER-46716: Add tests for let parameters for sharded update

Made cosmetic changes for the sake of consistency.

5 files changed, 32 insertions, 24 deletions

diff --git a/src/mongo/db/pipeline/aggregation_request.cpp b/src/mongo/db/pipeline/aggregation_request.cpp
index b75fb66a055..d2a7b9d0fbb 100644
--- a/src/mongo/db/pipeline/aggregation_request.cpp
+++ b/src/mongo/db/pipeline/aggregation_request.cpp
@@ -189,7 +189,7 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON(
 
             auto writeConcern = uassertStatusOK(WriteConcernOptions::parse(elem.embeddedObject()));
             request.setWriteConcern(writeConcern);
-        } else if (kRuntimeConstants == fieldName) {
+        } else if (kRuntimeConstantsName == fieldName) {
             // TODO SERVER-46384: Remove 'runtimeConstants' in 4.5 since it is redundant with 'let'
             try {
                 IDLParserErrorContext ctx("internalRuntimeConstants");
@@ -197,18 +197,18 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON(
             } catch (const DBException& ex) {
                 return ex.toStatus();
             }
-        } else if (kLet == fieldName) {
+        } else if (kLetName == fieldName) {
             if (elem.type() != BSONType::Object)
                 return {ErrorCodes::TypeMismatch,
                         str::stream()
                             << fieldName << " must be an object, not a " << typeName(elem.type())};
-            auto bob = BSONObjBuilder{request.letParameters};
+            auto bob = BSONObjBuilder{request.getLetParameters()};
             bob.appendElementsUnique(elem.embeddedObject());
-            request.letParameters = bob.obj();
-        } else if (fieldName == kUse44SortKeys) {
+            request._letParameters = bob.obj();
+        } else if (fieldName == kUse44SortKeysName) {
             if (elem.type() != BSONType::Bool) {
                 return {ErrorCodes::TypeMismatch,
-                        str::stream() << kUse44SortKeys << " must be a boolean, not a "
+                        str::stream() << kUse44SortKeysName << " must be a boolean, not a "
                                       << typeName(elem.type())};
             }
             // TODO SERVER-47065: A 4.6 node still has to accept the 'use44SortKeys' field, since it
@@ -219,10 +219,10 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON(
             // 4.6 upgrade purposes, since a 4.4 mongoS will always send {useNewUpsert:true} to the
             // shards. We do nothing with it because useNewUpsert will be automatically used in 4.6
             // when appropriate. Remove this final vestige of useNewUpsert during the 4.7 dev cycle.
-        } else if (fieldName == kIsMapReduceCommand) {
+        } else if (fieldName == kIsMapReduceCommandName) {
             if (elem.type() != BSONType::Bool) {
                 return {ErrorCodes::TypeMismatch,
-                        str::stream() << kIsMapReduceCommand << " must be a boolean, not a "
+                        str::stream() << kIsMapReduceCommandName << " must be a boolean, not a "
                                       << typeName(elem.type())};
             }
             request.setIsMapReduceCommand(elem.boolean());
@@ -327,9 +327,9 @@ Document AggregationRequest::serializeToCommandObj() const {
         {WriteConcernOptions::kWriteConcernField,
          _writeConcern ? Value(_writeConcern->toBSON()) : Value()},
         // Only serialize runtime constants if any were specified.
-        {kRuntimeConstants, _runtimeConstants ? Value(_runtimeConstants->toBSON()) : Value()},
-        {kIsMapReduceCommand, _isMapReduceCommand ? Value(true) : Value()},
-        {kLet, !letParameters.isEmpty() ? Value(letParameters) : Value()},
+        {kRuntimeConstantsName, _runtimeConstants ? Value(_runtimeConstants->toBSON()) : Value()},
+        {kIsMapReduceCommandName, _isMapReduceCommand ? Value(true) : Value()},
+        {kLetName, !_letParameters.isEmpty() ? Value(_letParameters) : Value()},
     };
 }
 }  // namespace mongo
diff --git a/src/mongo/db/pipeline/aggregation_request.h b/src/mongo/db/pipeline/aggregation_request.h
index 3e1de8412f6..7788ec962aa 100644
--- a/src/mongo/db/pipeline/aggregation_request.h
+++ b/src/mongo/db/pipeline/aggregation_request.h
@@ -62,10 +62,10 @@ public:
     static constexpr StringData kAllowDiskUseName = "allowDiskUse"_sd;
     static constexpr StringData kHintName = "hint"_sd;
     static constexpr StringData kExchangeName = "exchange"_sd;
-    static constexpr StringData kRuntimeConstants = "runtimeConstants"_sd;
-    static constexpr StringData kUse44SortKeys = "use44SortKeys"_sd;
-    static constexpr StringData kIsMapReduceCommand = "isMapReduceCommand"_sd;
-    static constexpr StringData kLet = "let"_sd;
+    static constexpr StringData kRuntimeConstantsName = "runtimeConstants"_sd;
+    static constexpr StringData kUse44SortKeysName = "use44SortKeys"_sd;
+    static constexpr StringData kIsMapReduceCommandName = "isMapReduceCommand"_sd;
+    static constexpr StringData kLetName = "let"_sd;
 
     static constexpr long long kDefaultBatchSize = 101;
 
@@ -219,6 +219,10 @@ public:
         return _runtimeConstants;
     }
 
+    const auto& getLetParameters() const {
+        return _letParameters;
+    }
+
     bool getIsMapReduceCommand() const {
         return _isMapReduceCommand;
     }
@@ -287,14 +291,14 @@ public:
         _runtimeConstants = std::move(runtimeConstants);
     }
 
+    void setLetParameters(BSONObj letParameters) {
+        _letParameters = letParameters.getOwned();
+    }
+
     void setIsMapReduceCommand(bool isMapReduce) {
         _isMapReduceCommand = isMapReduce;
     }
 
-    // A document containing user-specified let parameter constants; i.e. values that do not change
-    // once computed.
-    BSONObj letParameters;
-
 private:
     // Required fields.
     const NamespaceString _nss;
@@ -345,6 +349,10 @@ private:
     // $$NOW).
     boost::optional<RuntimeConstants> _runtimeConstants;
 
+    // A document containing user-specified let parameter constants; i.e. values that do not change
+    // once computed.
+    BSONObj _letParameters;
+
     // True when an aggregation was invoked by the MapReduce command.
     bool _isMapReduceCommand = false;
 };
diff --git a/src/mongo/db/pipeline/aggregation_request_test.cpp b/src/mongo/db/pipeline/aggregation_request_test.cpp
index 4618f9035bf..f16b7a5891b 100644
--- a/src/mongo/db/pipeline/aggregation_request_test.cpp
+++ b/src/mongo/db/pipeline/aggregation_request_test.cpp
@@ -192,7 +192,7 @@ TEST(AggregationRequestTest, ShouldSerializeOptionalValuesIfSet) {
     request.setIsMapReduceCommand(true);
     const auto letParamsObj = BSON("foo"
                                    << "bar");
-    request.letParameters = letParamsObj;
+    request.setLetParameters(letParamsObj);
 
     auto expectedSerialization =
         Document{{AggregationRequest::kCommandName, nss.coll()},
@@ -208,8 +208,8 @@ TEST(AggregationRequestTest, ShouldSerializeOptionalValuesIfSet) {
                  {repl::ReadConcernArgs::kReadConcernFieldName, readConcernObj},
                  {QueryRequest::kUnwrappedReadPrefField, readPrefObj},
                  {QueryRequest::cmdOptionMaxTimeMS, 10},
-                 {AggregationRequest::kIsMapReduceCommand, true},
-                 {AggregationRequest::kLet, letParamsObj}};
+                 {AggregationRequest::kIsMapReduceCommandName, true},
+                 {AggregationRequest::kLetName, letParamsObj}};
     ASSERT_DOCUMENT_EQ(request.serializeToCommandObj(), expectedSerialization);
 }
 
diff --git a/src/mongo/db/pipeline/expression_context.cpp b/src/mongo/db/pipeline/expression_context.cpp
index f33d6a16e06..bc5b3d26608 100644
--- a/src/mongo/db/pipeline/expression_context.cpp
+++ b/src/mongo/db/pipeline/expression_context.cpp
@@ -65,7 +65,7 @@ ExpressionContext::ExpressionContext(OperationContext* opCtx,
                         std::move(processInterface),
                         std::move(resolvedNamespaces),
                         std::move(collUUID),
-                        request.letParameters,
+                        request.getLetParameters(),
                         mayDbProfile) {
 
     if (request.getIsMapReduceCommand()) {
diff --git a/src/mongo/db/pipeline/sharded_agg_helpers.cpp b/src/mongo/db/pipeline/sharded_agg_helpers.cpp
index 7e02ec30fe3..1058c618cfd 100644
--- a/src/mongo/db/pipeline/sharded_agg_helpers.cpp
+++ b/src/mongo/db/pipeline/sharded_agg_helpers.cpp
@@ -127,7 +127,7 @@ BSONObj genericTransformForShards(MutableDocument&& cmdForShards,
                                   const boost::optional<RuntimeConstants>& constants,
                                   BSONObj collationObj) {
     if (constants) {
-        cmdForShards[AggregationRequest::kRuntimeConstants] = Value(constants.get().toBSON());
+        cmdForShards[AggregationRequest::kRuntimeConstantsName] = Value(constants.get().toBSON());
     }
 
     cmdForShards[AggregationRequest::kFromMongosName] = Value(expCtx->inMongos);