GODRIVER-526 Unskip keys in command monitoring assertions (#482)

Author: Divjot Arora

mongo/collection.go

@@ -279,7 +279,7 @@ func (coll *Collection) insert(ctx context.Context, documents []interface{},
 		Session(sess).WriteConcern(wc).CommandMonitor(coll.client.monitor).
 		ServerSelector(selector).ClusterClock(coll.client.clock).
 		Database(coll.db.name).Collection(coll.name).
-		Deployment(coll.client.deployment).Crypt(coll.client.crypt)
+		Deployment(coll.client.deployment).Crypt(coll.client.crypt).Ordered(true)
 	imo := options.MergeInsertManyOptions(opts...)
 	if imo.BypassDocumentValidation != nil && *imo.BypassDocumentValidation {
 		op = op.BypassDocumentValidation(*imo.BypassDocumentValidation)
@@ -454,7 +454,7 @@ func (coll *Collection) delete(ctx context.Context, filter interface{}, deleteOn
 		Session(sess).WriteConcern(wc).CommandMonitor(coll.client.monitor).
 		ServerSelector(selector).ClusterClock(coll.client.clock).
 		Database(coll.db.name).Collection(coll.name).
-		Deployment(coll.client.deployment).Crypt(coll.client.crypt)
+		Deployment(coll.client.deployment).Crypt(coll.client.crypt).Ordered(true)
 	if do.Hint != nil {
 		op = op.Hint(true)
 	}
@@ -551,7 +551,7 @@ func (coll *Collection) updateOrReplace(ctx context.Context, filter bsoncore.Doc
 		ServerSelector(selector).ClusterClock(coll.client.clock).
 		Database(coll.db.name).Collection(coll.name).
 		Deployment(coll.client.deployment).Crypt(coll.client.crypt).Hint(uo.Hint != nil).
-		ArrayFilters(uo.ArrayFilters != nil)
+		ArrayFilters(uo.ArrayFilters != nil).Ordered(true)
 
 	if uo.BypassDocumentValidation != nil && *uo.BypassDocumentValidation {
 		op = op.BypassDocumentValidation(*uo.BypassDocumentValidation)

mongo/database.go

@@ -363,6 +363,10 @@ func (db *Database) ListCollections(ctx context.Context, filter interface{}, opt
 	if lco.NameOnly != nil {
 		op = op.NameOnly(*lco.NameOnly)
 	}
+	if lco.BatchSize != nil {
+		op = op.BatchSize(*lco.BatchSize)
+	}
+
 	retry := driver.RetryNone
 	if db.client.retryReads {
 		retry = driver.RetryOncePerCommand

mongo/integration/cmd_monitoring_helpers_test.go

@@ -253,26 +253,21 @@ func compareStartedEvent(mt *mtest.T, expectation *expectation, id0, id1 bson.Ra
 		key := elem.Key()
 		val := elem.Value()
 
-		actualVal := evt.Command.Lookup(key)
+		actualVal, err := evt.Command.LookupErr(key)
 
 		// Keys that may be nil
 		if val.Type == bson.TypeNull {
-			if actualVal.Type != 0 || len(actualVal.Value) > 0 {
-				return fmt.Errorf("expected value for key %s to be nil but got %s", key, actualVal)
-			}
-			continue
-		}
-		if key == "ordered" || key == "cursor" || key == "batchSize" {
-			// TODO: some tests specify that "ordered" must be a key in the event but ordered isn't a valid option for
-			// some of these cases (e.g. insertOne)
-			// TODO: some FLE tests specify "cursor" subdocument for listCollections
-			// TODO: find.json cmd monitoring tests expect different batch sizes for find/getMore commands based on an
-			// optional limit
+			assert.NotNil(mt, err, "expected key %q to be omitted but got %q", key, actualVal)
 			continue
 		}
+		assert.Nil(mt, err, "expected command to contain key %q", key)
 
-		if err = actualVal.Validate(); err != nil {
-			return fmt.Errorf("error validatinmg value for key %s: %s", key, err)
+		if key == "batchSize" {
+			// Some command monitoring tests expect that the driver will send a lower batch size if the required batch
+			// size is lower than the operation limit. We only do this for legacy servers <= 3.0 because those server
+			// versions do not support the limit option, but not for 3.2+. We've already validated that the command
+			// contains a batchSize field above and we can skip the actual value comparison below.
+			continue
 		}
 
 		switch key {

mongo/integration/database_test.go

@@ -137,45 +137,66 @@ func TestDatabase(t *testing.T) {
 	})
 
 	mt.RunOpts("list collections", noClientOpts, func(mt *mtest.T) {
-		testCases := []struct {
-			name             string
-			expectedTopology mtest.TopologyKind
-			cappedOnly       bool
-		}{
-			{"standalone no filter", mtest.Single, false},
-			{"standalone filter", mtest.Single, true},
-			{"replica set no filter", mtest.ReplicaSet, false},
-			{"replica set filter", mtest.ReplicaSet, true},
-			{"sharded no filter", mtest.Sharded, false},
-			{"sharded filter", mtest.Sharded, true},
-		}
-		for _, tc := range testCases {
-			tcOpts := mtest.NewOptions().Topologies(tc.expectedTopology)
-			mt.RunOpts(tc.name, tcOpts, func(mt *mtest.T) {
-				mt.CreateCollection(mtest.Collection{Name: listCollUncapped}, true)
-				mt.CreateCollection(mtest.Collection{
-					Name:       listCollCapped,
-					CreateOpts: bson.D{{"capped", true}, {"size", 64 * 1024}},
-				}, true)
+		mt.RunOpts("verify results", noClientOpts, func(mt *mtest.T) {
+			testCases := []struct {
+				name             string
+				expectedTopology mtest.TopologyKind
+				cappedOnly       bool
+			}{
+				{"standalone no filter", mtest.Single, false},
+				{"standalone filter", mtest.Single, true},
+				{"replica set no filter", mtest.ReplicaSet, false},
+				{"replica set filter", mtest.ReplicaSet, true},
+				{"sharded no filter", mtest.Sharded, false},
+				{"sharded filter", mtest.Sharded, true},
+			}
+			for _, tc := range testCases {
+				tcOpts := mtest.NewOptions().Topologies(tc.expectedTopology)
+				mt.RunOpts(tc.name, tcOpts, func(mt *mtest.T) {
+					mt.CreateCollection(mtest.Collection{Name: listCollUncapped}, true)
+					mt.CreateCollection(mtest.Collection{
+						Name:       listCollCapped,
+						CreateOpts: bson.D{{"capped", true}, {"size", 64 * 1024}},
+					}, true)
 
-				filter := bson.D{}
-				if tc.cappedOnly {
-					filter = bson.D{{"options.capped", true}}
-				}
+					filter := bson.D{}
+					if tc.cappedOnly {
+						filter = bson.D{{"options.capped", true}}
+					}
 
-				var err error
-				for i := 0; i < 1; i++ {
-					cursor, err := mt.DB.ListCollections(mtest.Background, filter)
-					assert.Nil(mt, err, "ListCollections error (iteration %v): %v", i, err)
+					var err error
+					for i := 0; i < 1; i++ {
+						cursor, err := mt.DB.ListCollections(mtest.Background, filter)
+						assert.Nil(mt, err, "ListCollections error (iteration %v): %v", i, err)
 
-					err = verifyListCollections(cursor, tc.cappedOnly)
-					if err == nil {
-						return
+						err = verifyListCollections(cursor, tc.cappedOnly)
+						if err == nil {
+							return
+						}
 					}
-				}
-				mt.Fatalf("error verifying list collections result: %v", err)
-			})
-		}
+					mt.Fatalf("error verifying list collections result: %v", err)
+				})
+			}
+		})
+		mt.RunOpts("batch size", mtest.NewOptions().MinServerVersion("3.0"), func(mt *mtest.T) {
+			// Create two new collections so there will be three total.
+			for i := 0; i < 2; i++ {
+				mt.CreateCollection(mtest.Collection{
+					Name: fmt.Sprintf("list-collections-batchSize-%d", i),
+				}, true)
+			}
+
+			mt.ClearEvents()
+			lcOpts := options.ListCollections().SetBatchSize(2)
+			_, err := mt.DB.ListCollectionNames(mtest.Background, bson.D{}, lcOpts)
+			assert.Nil(mt, err, "ListCollectionNames error: %v", err)
+
+			evt := mt.GetStartedEvent()
+			assert.Equal(mt, "listCollections", evt.CommandName, "expected 'listCollections' command to be sent, got %q",
+				evt.CommandName)
+			_, err = evt.Command.LookupErr("cursor", "batchSize")
+			assert.Nil(mt, err, "expected command %s to contain key 'batchSize'", evt.Command)
+		})
 	})
 
 	mt.RunOpts("list collection specifications", noClientOpts, func(mt *mtest.T) {

mongo/options/listcollectionsoptions.go

@@ -10,6 +10,9 @@ package options
 type ListCollectionsOptions struct {
 	// If true, each collection document will only contain a field for the collection name. The default value is false.
 	NameOnly *bool
+
+	// The maximum number of documents to be included in each batch returned by the server.
+	BatchSize *int32
 }
 
 // ListCollections creates a new ListCollectionsOptions instance.
@@ -23,6 +26,12 @@ func (lc *ListCollectionsOptions) SetNameOnly(b bool) *ListCollectionsOptions {
 	return lc
 }
 
+// SetBatchSize sets the value for the BatchSize field.
+func (lc *ListCollectionsOptions) SetBatchSize(size int32) *ListCollectionsOptions {
+	lc.BatchSize = &size
+	return lc
+}
+
 // MergeListCollectionsOptions combines the given ListCollectionsOptions instances into a single *ListCollectionsOptions
 // in a last-one-wins fashion.
 func MergeListCollectionsOptions(opts ...*ListCollectionsOptions) *ListCollectionsOptions {
@@ -34,6 +43,9 @@ func MergeListCollectionsOptions(opts ...*ListCollectionsOptions) *ListCollectio
 		if opt.NameOnly != nil {
 			lc.NameOnly = opt.NameOnly
 		}
+		if opt.BatchSize != nil {
+			lc.BatchSize = opt.BatchSize
+		}
 	}
 
 	return lc