SERVER-24771 Use view namespace in cursors for aggregate/getMore

6 files changed, 147 insertions, 59 deletions

diff --git a/jstests/auth/views_authz.js b/jstests/auth/views_authz.js
index e187dff9c90..4a851583110 100644
--- a/jstests/auth/views_authz.js
+++ b/jstests/auth/views_authz.js
@@ -26,6 +26,7 @@
               resource: {db: viewsDBName, collection: "view"},
               actions: ["find", "createCollection", "collMod"]
             },
+            {resource: {db: viewsDBName, collection: "view2"}, actions: ["find"]},
             {resource: {db: viewsDBName, collection: "permitted"}, actions: ["find"]}
         ],
         roles: []
@@ -83,4 +84,18 @@
         viewsDB.runCommand(
             {collMod: "view", viewOn: "permitted", pipeline: [{$facet: {b: [graphLookupStage]}}]}),
         ErrorCodes.Unauthorized);
+
+    // Performing a find on a readable view returns a cursor that allows us to perform a getMore
+    // even if the underlying collection is unreadable.
+    assert.eq(1, adminDB.auth("admin", "admin"));
+    assert.commandWorked(viewsDB.createView("view2", "forbidden", []));
+    for (let i = 0; i < 10; i++) {
+        assert.writeOK(viewsDB.forbidden.insert({x: 1}));
+    }
+    adminDB.logout();
+    assert.commandFailedWithCode(viewsDB.runCommand({find: "forbidden"}), ErrorCodes.Unauthorized);
+    let res = viewsDB.runCommand({find: "view2", batchSize: 1});
+    assert.commandWorked(res);
+    assert.eq(res.cursor.ns, "views_authz.view2");
+    assert.commandWorked(viewsDB.runCommand({getMore: res.cursor.id, collection: "view2"}));
 }());
diff --git a/jstests/views/views_basic.js b/jstests/views/views_basic.js
index b5459380f07..d6032b105df 100644
--- a/jstests/views/views_basic.js
+++ b/jstests/views/views_basic.js
@@ -14,7 +14,9 @@
         assert.commandWorked(res);
 
         let cursor = new DBCommandCursor(db.getMongo(), res, 5);
-        assert(arrayEq(cursor.toArray(), expected));
+        let actual = cursor.toArray();
+        assert(arrayEq(actual, expected),
+               "actual: " + tojson(cursor.toArray()) + ", expected:" + tojson(expected));
     };
 
     // Insert some control documents.
diff --git a/src/mongo/db/commands/getmore_cmd.cpp b/src/mongo/db/commands/getmore_cmd.cpp
index 2fcf53e8e82..cbcf44f19d0 100644
--- a/src/mongo/db/commands/getmore_cmd.cpp
+++ b/src/mongo/db/commands/getmore_cmd.cpp
@@ -145,26 +145,12 @@ public:
             request.nss, request.cursorid, request.term.is_initialized());
     }
 
-    bool run(OperationContext* txn,
-             const std::string& dbname,
-             BSONObj& cmdObj,
-             int options,
-             std::string& errmsg,
-             BSONObjBuilder& result) override {
-        // Counted as a getMore, not as a command.
-        globalOpCounters.gotGetMore();
-
-        if (txn->getClient()->isInDirectClient()) {
-            return appendCommandStatus(
-                result,
-                Status(ErrorCodes::IllegalOperation, "Cannot run getMore command from eval()"));
-        }
-
-        StatusWith<GetMoreRequest> parseStatus = GetMoreRequest::parseFromBSON(dbname, cmdObj);
-        if (!parseStatus.isOK()) {
-            return appendCommandStatus(result, parseStatus.getStatus());
-        }
-        const GetMoreRequest& request = parseStatus.getValue();
+    bool runParsed(OperationContext* txn,
+                   const NamespaceString& origNss,
+                   const GetMoreRequest& request,
+                   BSONObj& cmdObj,
+                   std::string& errmsg,
+                   BSONObjBuilder& result) {
 
         auto curOp = CurOp::get(txn);
         curOp->debug().cursorid = request.cursorid;
@@ -208,9 +194,41 @@ public:
         if (request.nss.isListIndexesCursorNS() || request.nss.isListCollectionsCursorNS()) {
             cursorManager = CursorManager::getGlobalCursorManager();
         } else {
-            ctx = stdx::make_unique<AutoGetCollectionForRead>(txn, request.nss);
+            ctx = stdx::make_unique<AutoGetCollectionOrViewForRead>(txn, request.nss);
+            auto viewCtx = static_cast<AutoGetCollectionOrViewForRead*>(ctx.get());
             Collection* collection = ctx->getCollection();
             if (!collection) {
+                // Rewrite a getMore on a view to a getMore on the original underlying collection.
+                // If the view no longer exists, or has been rewritten, the cursor id will be
+                // unknown, resulting in an appropriate error.
+                if (viewCtx->getView()) {
+                    auto resolved =
+                        viewCtx->getDb()->getViewCatalog()->resolveView(txn, request.nss);
+                    if (!resolved.isOK()) {
+                        return appendCommandStatus(result, resolved.getStatus());
+                    }
+                    viewCtx->releaseLocksForView();
+
+                    // Only one shardversion can be set at a time for an operation, so unset it
+                    // here to allow setting it on the underlying namespace.
+                    OperationShardingState::get(txn).unsetShardVersion(request.nss);
+
+                    GetMoreRequest newRequest(resolved.getValue().getNamespace(),
+                                              request.cursorid,
+                                              request.batchSize,
+                                              request.awaitDataTimeout,
+                                              request.term,
+                                              request.lastKnownCommittedOpTime);
+
+                    bool retVal = runParsed(txn, origNss, newRequest, cmdObj, errmsg, result);
+                    {
+                        // Set the namespace of the curop back to the view namespace so ctx records
+                        // stats on this view namespace on destruction.
+                        stdx::lock_guard<Client>(*txn->getClient());
+                        curOp->setNS_inlock(origNss.ns());
+                    }
+                    return retVal;
+                }
                 return appendCommandStatus(result,
                                            Status(ErrorCodes::OperationFailed,
                                                   "collection dropped between getMore calls"));
@@ -429,7 +447,9 @@ public:
             curOp->debug().cursorExhausted = true;
         }
 
-        nextBatch.done(respondWithId, request.nss.ns());
+        // Respond with the originally requested namespace, even if this is a getMore over a view
+        // that was resolved to a different backing namespace.
+        nextBatch.done(respondWithId, origNss.ns());
 
         // Ensure log and profiler include the number of results returned in this getMore's response
         // batch.
@@ -451,6 +471,29 @@ public:
         return true;
     }
 
+    bool run(OperationContext* txn,
+             const std::string& dbname,
+             BSONObj& cmdObj,
+             int options,
+             std::string& errmsg,
+             BSONObjBuilder& result) override {
+        // Counted as a getMore, not as a command.
+        globalOpCounters.gotGetMore();
+
+        if (txn->getClient()->isInDirectClient()) {
+            return appendCommandStatus(
+                result,
+                Status(ErrorCodes::IllegalOperation, "Cannot run getMore command from eval()"));
+        }
+
+        StatusWith<GetMoreRequest> parsedRequest = GetMoreRequest::parseFromBSON(dbname, cmdObj);
+        if (!parsedRequest.isOK()) {
+            return appendCommandStatus(result, parsedRequest.getStatus());
+        }
+        auto request = parsedRequest.getValue();
+        return runParsed(txn, request.nss, request, cmdObj, errmsg, result);
+    }
+
     /**
      * Uses 'cursor' and 'request' to fill out 'nextBatch' with the batch of result documents to
      * be returned by this getMore.
diff --git a/src/mongo/db/commands/pipeline_command.cpp b/src/mongo/db/commands/pipeline_command.cpp
index 2aa42c8f736..4f64276be72 100644
--- a/src/mongo/db/commands/pipeline_command.cpp
+++ b/src/mongo/db/commands/pipeline_command.cpp
@@ -79,10 +79,12 @@ namespace {
 
 /**
  * Returns true if we need to keep a ClientCursor saved for this pipeline (for future getMore
- * requests).  Otherwise, returns false.
+ * requests). Otherwise, returns false. The passed 'nsForCursor' is only used to determine the
+ * namespace used in the returned cursor. In the case of views, this can be different from that
+ * in 'request'.
  */
 bool handleCursorCommand(OperationContext* txn,
-                         const string& ns,
+                         const string& nsForCursor,
                          ClientCursorPin* pin,
                          PlanExecutor* exec,
                          const AggregationRequest& request,
@@ -139,7 +141,7 @@ bool handleCursorCommand(OperationContext* txn,
             17391,
             str::stream() << "Aggregation has more results than fit in initial batch, but can't "
                           << "create cursor since collection "
-                          << ns
+                          << nsForCursor
                           << " doesn't exist");
     }
 
@@ -159,7 +161,7 @@ bool handleCursorCommand(OperationContext* txn,
     }
 
     const long long cursorId = cursor ? cursor->cursorid() : 0LL;
-    appendCursorResponseObject(cursorId, ns, resultsArray.arr(), &result);
+    appendCursorResponseObject(cursorId, nsForCursor, resultsArray.arr(), &result);
 
     return static_cast<bool>(cursor);
 }
@@ -303,31 +305,21 @@ public:
         return Pipeline::checkAuthForCommand(client, dbname, cmdObj);
     }
 
-    virtual bool run(OperationContext* txn,
-                     const string& db,
-                     BSONObj& cmdObj,
-                     int options,
-                     string& errmsg,
-                     BSONObjBuilder& result) {
-        const std::string ns = parseNs(db, cmdObj);
-        if (nsToCollectionSubstring(ns).empty()) {
-            errmsg = "missing collection name";
-            return false;
-        }
-        NamespaceString nss(ns);
-
-        // Parse the options for this request.
-        auto request = AggregationRequest::parseFromBSON(nss, cmdObj);
-        if (!request.isOK()) {
-            return appendCommandStatus(result, request.getStatus());
-        }
+    bool runParsed(OperationContext* txn,
+                   const NamespaceString& origNss,
+                   const AggregationRequest& request,
+                   BSONObj& cmdObj,
+                   string& errmsg,
+                   BSONObjBuilder& result) {
+        // For operations on views, this will be the underlying namespace.
+        const NamespaceString& nss = request.getNamespaceString();
 
         // Set up the ExpressionContext.
-        intrusive_ptr<ExpressionContext> expCtx = new ExpressionContext(txn, request.getValue());
+        intrusive_ptr<ExpressionContext> expCtx = new ExpressionContext(txn, request);
         expCtx->tempDir = storageGlobalParams.dbpath + "/_tmp";
 
         // Parse the pipeline.
-        auto statusWithPipeline = Pipeline::parse(request.getValue().getPipeline(), expCtx);
+        auto statusWithPipeline = Pipeline::parse(request.getPipeline(), expCtx);
         if (!statusWithPipeline.isOK()) {
             return appendCommandStatus(result, statusWithPipeline.getStatus());
         }
@@ -388,13 +380,18 @@ public:
                 ctx.releaseLocksForView();
 
                 // Parse the resolved view into a new aggregation request.
-                auto viewCmd =
-                    resolvedView.getValue().asExpandedViewAggregation(request.getValue());
-                if (!viewCmd.isOK()) {
-                    return appendCommandStatus(result, viewCmd.getStatus());
+                auto newCmd = resolvedView.getValue().asExpandedViewAggregation(request);
+                if (!newCmd.isOK()) {
+                    return appendCommandStatus(result, newCmd.getStatus());
+                }
+                auto newNss = resolvedView.getValue().getNamespace();
+                auto newRequest = AggregationRequest::parseFromBSON(newNss, newCmd.getValue());
+                if (!newRequest.isOK()) {
+                    return appendCommandStatus(result, newRequest.getStatus());
                 }
 
-                bool status = this->run(txn, db, viewCmd.getValue(), options, errmsg, result);
+                bool status = runParsed(
+                    txn, origNss, newRequest.getValue(), newCmd.getValue(), errmsg, result);
                 {
                     // Set the namespace of the curop back to the view namespace so ctx records
                     // stats on this view namespace on destruction.
@@ -406,7 +403,7 @@ public:
 
             // If the pipeline does not have a user-specified collation, set it from the collection
             // default.
-            if (request.getValue().getCollation().isEmpty() && collection &&
+            if (request.getCollation().isEmpty() && collection &&
                 collection->getDefaultCollator()) {
                 invariant(!expCtx->getCollator());
                 expCtx->setCollator(collection->getDefaultCollator()->clone());
@@ -427,7 +424,7 @@ public:
                 // re-parsing every command in debug builds. This is important because sharded
                 // aggregations rely on this ability.  Skipping when inShard because this has
                 // already been through the transformation (and this un-sets expCtx->inShard).
-                pipeline = reparsePipeline(pipeline, request.getValue(), expCtx);
+                pipeline = reparsePipeline(pipeline, request, expCtx);
             }
 
             // This does mongod-specific stuff like creating the input PlanExecutor and adding
@@ -492,7 +489,7 @@ public:
             // BSONObj will also fit inside a single batch.
             //
             // We occasionally log a deprecation warning.
-            if (!request.getValue().isCursorCommand()) {
+            if (!request.isCursorCommand()) {
                 RARELY {
                     warning()
                         << "Use of the aggregate command without the 'cursor' "
@@ -504,12 +501,12 @@ public:
             // If both explain and cursor are specified, explain wins.
             if (expCtx->isExplain) {
                 result << "stages" << Value(pipeline->writeExplainOps());
-            } else if (request.getValue().isCursorCommand()) {
+            } else if (request.isCursorCommand()) {
                 keepCursor = handleCursorCommand(txn,
-                                                 nss.ns(),
+                                                 origNss.ns(),
                                                  pin.get(),
                                                  pin ? pin->c()->getExecutor() : exec.get(),
-                                                 request.getValue(),
+                                                 request,
                                                  result);
             } else {
                 pipeline->run(result);
@@ -547,9 +544,30 @@ public:
             throw;
         }
         // Any code that needs the cursor pinned must be inside the try block, above.
-
         return appendCommandStatus(result, Status::OK());
     }
+
+    virtual bool run(OperationContext* txn,
+                     const string& db,
+                     BSONObj& cmdObj,
+                     int options,
+                     string& errmsg,
+                     BSONObjBuilder& result) {
+        const std::string ns = parseNs(db, cmdObj);
+        if (nsToCollectionSubstring(ns).empty()) {
+            errmsg = "missing collection name";
+            return false;
+        }
+        NamespaceString nss(ns);
+
+        // Parse the options for this request.
+        auto request = AggregationRequest::parseFromBSON(nss, cmdObj);
+        if (!request.isOK()) {
+            return appendCommandStatus(result, request.getStatus());
+        }
+
+        return runParsed(txn, nss, request.getValue(), cmdObj, errmsg, result);
+    }
 };
 
 MONGO_INITIALIZER(PipelineCommand)(InitializerContext* context) {
diff --git a/src/mongo/db/s/operation_sharding_state.cpp b/src/mongo/db/s/operation_sharding_state.cpp
index b1ba9e0e16f..a940ffc01df 100644
--- a/src/mongo/db/s/operation_sharding_state.cpp
+++ b/src/mongo/db/s/operation_sharding_state.cpp
@@ -90,6 +90,11 @@ void OperationShardingState::setShardVersion(NamespaceString nss, ChunkVersion n
     _hasVersion = true;
 }
 
+void OperationShardingState::unsetShardVersion(NamespaceString nss) {
+    invariant(!_hasVersion || _ns == nss);
+    _clear();
+}
+
 bool OperationShardingState::waitForMigrationCriticalSectionSignal(OperationContext* txn) {
     // Must not block while holding a lock
     invariant(!txn->lockState()->isLocked());
diff --git a/src/mongo/db/s/operation_sharding_state.h b/src/mongo/db/s/operation_sharding_state.h
index d83822d5696..aa03834da6a 100644
--- a/src/mongo/db/s/operation_sharding_state.h
+++ b/src/mongo/db/s/operation_sharding_state.h
@@ -93,6 +93,11 @@ public:
     void setShardVersion(NamespaceString nss, ChunkVersion newVersion);
 
     /**
+     * Undoes setting the shard version for the given namespace. Needed for views.
+     */
+    void unsetShardVersion(NamespaceString nss);
+
+    /**
      * This call is a no op if there isn't a currently active migration critical section. Otherwise
      * it will wait for the critical section to complete up to the remaining operation time.
      *