From ff57da9a487f985d16016f6c44d20a23743dd31d Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Mon, 28 Aug 2023 16:32:17 -0700 Subject: [PATCH] GODRIVER-2962 Remove setters from Command. --- internal/integtest/integtest.go | 19 +- mongo/database.go | 33 ++- x/mongo/driver/auth/mongodbcr.go | 24 +- x/mongo/driver/auth/sasl.go | 24 +- x/mongo/driver/auth/x509.go | 13 +- x/mongo/driver/integration/compressor_test.go | 8 +- x/mongo/driver/integration/main_test.go | 17 +- x/mongo/driver/operation/command.go | 232 +++++------------- 8 files changed, 151 insertions(+), 219 deletions(-) diff --git a/internal/integtest/integtest.go b/internal/integtest/integtest.go index d89bcd7539..ead9488c7e 100644 --- a/internal/integtest/integtest.go +++ b/internal/integtest/integtest.go @@ -101,9 +101,13 @@ func MonitoredTopology(t *testing.T, dbName string, monitor *event.CommandMonito } else { _ = monitoredTopology.Connect() - err = operation.NewCommand(bsoncore.BuildDocument(nil, bsoncore.AppendInt32Element(nil, "dropDatabase", 1))). - Database(dbName).ServerSelector(description.WriteSelector()).Deployment(monitoredTopology).Execute(context.Background()) - + op := &operation.Command{ + Command: bsoncore.BuildDocument(nil, bsoncore.AppendInt32Element(nil, "dropDatabase", 1)), + Database: dbName, + Selector: description.WriteSelector(), + Deployment: monitoredTopology, + } + err = op.Execute(context.Background()) require.NoError(t, err) } @@ -125,8 +129,13 @@ func Topology(t *testing.T) *topology.Topology { } else { _ = liveTopology.Connect() - err = operation.NewCommand(bsoncore.BuildDocument(nil, bsoncore.AppendInt32Element(nil, "dropDatabase", 1))). - Database(DBName(t)).ServerSelector(description.WriteSelector()).Deployment(liveTopology).Execute(context.Background()) + op := &operation.Command{ + Command: bsoncore.BuildDocument(nil, bsoncore.AppendInt32Element(nil, "dropDatabase", 1)), + Database: DBName(t), + Selector: description.WriteSelector(), + Deployment: liveTopology, + } + err = op.Execute(context.Background()) require.NoError(t, err) } }) diff --git a/mongo/database.go b/mongo/database.go index f5d5ad379b..7e9a0353b9 100644 --- a/mongo/database.go +++ b/mongo/database.go @@ -180,19 +180,32 @@ func (db *Database) processRunCommand(ctx context.Context, cmd interface{}, cursorOpts.MarshalValueEncoderFn = newEncoderFn(db.bsonOpts, db.registry) - op = operation.NewCursorCommand(runCmdDoc, cursorOpts) + op = &operation.Command{ + Command: runCmdDoc, + CreateCursor: true, + CursorOpts: cursorOpts, + } default: - op = operation.NewCommand(runCmdDoc) + op = &operation.Command{ + Command: runCmdDoc, + } } - // TODO(GODRIVER-2649): ReadConcern(db.readConcern) will not actually pass the database's - // read concern. Remove this note once readConcern is correctly passed to the operation - // level. - return op.Session(sess).CommandMonitor(db.client.monitor). - ServerSelector(readSelect).ClusterClock(db.client.clock). - Database(db.name).Deployment(db.client.deployment).ReadConcern(db.readConcern). - Crypt(db.client.cryptFLE).ReadPreference(ro.ReadPreference).ServerAPI(db.client.serverAPI). - Timeout(db.client.timeout).Logger(db.client.logger), sess, nil + // TODO(GODRIVER-2649): Set read concern on the operation once the Command + // TODO type actually supports it. + op.Session = sess + op.Monitor = db.client.monitor + op.Selector = readSelect + op.Clock = db.client.clock + op.Database = db.name + op.Deployment = db.client.deployment + op.Crypt = db.client.cryptFLE + op.ReadPreference = ro.ReadPreference + op.ServerAPI = db.client.serverAPI + op.Timeout = db.client.timeout + op.Logger = db.client.logger + + return op, sess, nil } // RunCommand executes the given command against the database. This function does not obey the Database's read diff --git a/x/mongo/driver/auth/mongodbcr.go b/x/mongo/driver/auth/mongodbcr.go index 6e2c2f4dcb..932cd47ac8 100644 --- a/x/mongo/driver/auth/mongodbcr.go +++ b/x/mongo/driver/auth/mongodbcr.go @@ -58,11 +58,13 @@ func (a *MongoDBCRAuthenticator) Auth(ctx context.Context, cfg *Config) error { } doc := bsoncore.BuildDocumentFromElements(nil, bsoncore.AppendInt32Element(nil, "getnonce", 1)) - cmd := operation.NewCommand(doc). - Database(db). - Deployment(driver.SingleConnectionDeployment{cfg.Connection}). - ClusterClock(cfg.ClusterClock). - ServerAPI(cfg.ServerAPI) + cmd := &operation.Command{ + Command: doc, + Database: db, + Deployment: driver.SingleConnectionDeployment{cfg.Connection}, + Clock: cfg.ClusterClock, + ServerAPI: cfg.ServerAPI, + } err := cmd.Execute(ctx) if err != nil { return newError(err, MONGODBCR) @@ -84,11 +86,13 @@ func (a *MongoDBCRAuthenticator) Auth(ctx context.Context, cfg *Config) error { bsoncore.AppendStringElement(nil, "nonce", getNonceResult.Nonce), bsoncore.AppendStringElement(nil, "key", a.createKey(getNonceResult.Nonce)), ) - cmd = operation.NewCommand(doc). - Database(db). - Deployment(driver.SingleConnectionDeployment{cfg.Connection}). - ClusterClock(cfg.ClusterClock). - ServerAPI(cfg.ServerAPI) + cmd = &operation.Command{ + Command: doc, + Database: db, + Deployment: driver.SingleConnectionDeployment{cfg.Connection}, + Clock: cfg.ClusterClock, + ServerAPI: cfg.ServerAPI, + } err = cmd.Execute(ctx) if err != nil { return newError(err, MONGODBCR) diff --git a/x/mongo/driver/auth/sasl.go b/x/mongo/driver/auth/sasl.go index a7ae3368f0..41196aa879 100644 --- a/x/mongo/driver/auth/sasl.go +++ b/x/mongo/driver/auth/sasl.go @@ -132,11 +132,13 @@ func (sc *saslConversation) Finish(ctx context.Context, cfg *Config, firstRespon bsoncore.AppendInt32Element(nil, "conversationId", int32(cid)), bsoncore.AppendBinaryElement(nil, "payload", 0x00, payload), ) - saslContinueCmd := operation.NewCommand(doc). - Database(sc.source). - Deployment(driver.SingleConnectionDeployment{cfg.Connection}). - ClusterClock(cfg.ClusterClock). - ServerAPI(cfg.ServerAPI) + saslContinueCmd := &operation.Command{ + Command: doc, + Database: sc.source, + Deployment: driver.SingleConnectionDeployment{C: cfg.Connection}, + Clock: cfg.ClusterClock, + ServerAPI: cfg.ServerAPI, + } err = saslContinueCmd.Execute(ctx) if err != nil { @@ -161,11 +163,13 @@ func ConductSaslConversation(ctx context.Context, cfg *Config, authSource string if err != nil { return newError(err, conversation.mechanism) } - saslStartCmd := operation.NewCommand(saslStartDoc). - Database(authSource). - Deployment(driver.SingleConnectionDeployment{cfg.Connection}). - ClusterClock(cfg.ClusterClock). - ServerAPI(cfg.ServerAPI) + saslStartCmd := &operation.Command{ + Command: saslStartDoc, + Database: authSource, + Deployment: driver.SingleConnectionDeployment{C: cfg.Connection}, + Clock: cfg.ClusterClock, + ServerAPI: cfg.ServerAPI, + } if err := saslStartCmd.Execute(ctx); err != nil { return newError(err, conversation.mechanism) } diff --git a/x/mongo/driver/auth/x509.go b/x/mongo/driver/auth/x509.go index 03a9d750e2..4763ba2ec3 100644 --- a/x/mongo/driver/auth/x509.go +++ b/x/mongo/driver/auth/x509.go @@ -63,12 +63,13 @@ func (a *MongoDBX509Authenticator) CreateSpeculativeConversation() (SpeculativeC // Auth authenticates the provided connection by conducting an X509 authentication conversation. func (a *MongoDBX509Authenticator) Auth(ctx context.Context, cfg *Config) error { requestDoc := createFirstX509Message() - authCmd := operation. - NewCommand(requestDoc). - Database("$external"). - Deployment(driver.SingleConnectionDeployment{cfg.Connection}). - ClusterClock(cfg.ClusterClock). - ServerAPI(cfg.ServerAPI) + authCmd := &operation.Command{ + Command: requestDoc, + Database: "$external", + Deployment: driver.SingleConnectionDeployment{C: cfg.Connection}, + Clock: cfg.ClusterClock, + ServerAPI: cfg.ServerAPI, + } err := authCmd.Execute(ctx) if err != nil { return newAuthError("round trip error", err) diff --git a/x/mongo/driver/integration/compressor_test.go b/x/mongo/driver/integration/compressor_test.go index 16f809cb80..1a68f0c418 100644 --- a/x/mongo/driver/integration/compressor_test.go +++ b/x/mongo/driver/integration/compressor_test.go @@ -33,9 +33,11 @@ func TestCompression(t *testing.T) { bsoncore.BuildDocument(nil, bsoncore.AppendStringElement(nil, "name", "compression_test")), ) - cmd := operation.NewCommand(bsoncore.BuildDocument(nil, bsoncore.AppendInt32Element(nil, "serverStatus", 1))). - Deployment(integtest.Topology(t)). - Database(integtest.DBName(t)) + cmd := &operation.Command{ + Command: bsoncore.BuildDocument(nil, bsoncore.AppendInt32Element(nil, "serverStatus", 1)), + Deployment: integtest.Topology(t), + Database: integtest.DBName(t), + } ctx := context.Background() err := cmd.Execute(ctx) diff --git a/x/mongo/driver/integration/main_test.go b/x/mongo/driver/integration/main_test.go index ef6331853d..0ce6072e03 100644 --- a/x/mongo/driver/integration/main_test.go +++ b/x/mongo/driver/integration/main_test.go @@ -122,8 +122,11 @@ func addCompressorToURI(uri string) string { // runCommand runs an arbitrary command on a given database of target server func runCommand(s driver.Server, db string, cmd bsoncore.Document) (bsoncore.Document, error) { - op := operation.NewCommand(cmd). - Database(db).Deployment(driver.SingleServerDeployment{Server: s}) + op := &operation.Command{ + Command: cmd, + Database: db, + Deployment: driver.SingleServerDeployment{Server: s}, + } err := op.Execute(context.Background()) res := op.Result() return res, err @@ -131,9 +134,13 @@ func runCommand(s driver.Server, db string, cmd bsoncore.Document) (bsoncore.Doc // dropCollection drops the collection in the test cluster. func dropCollection(t *testing.T, dbname, colname string) { - err := operation.NewCommand(bsoncore.BuildDocument(nil, bsoncore.AppendStringElement(nil, "drop", colname))). - Database(dbname).ServerSelector(description.WriteSelector()).Deployment(integtest.Topology(t)). - Execute(context.Background()) + op := &operation.Command{ + Command: bsoncore.BuildDocument(nil, bsoncore.AppendStringElement(nil, "drop", colname)), + Database: dbname, + Selector: description.WriteSelector(), + Deployment: integtest.Topology(t), + } + err := op.Execute(context.Background()) if de, ok := err.(driver.Error); err != nil && !(ok && de.NamespaceNotFound()) { require.NoError(t, err) } diff --git a/x/mongo/driver/operation/command.go b/x/mongo/driver/operation/command.go index 5aad3f72e6..7df0988983 100644 --- a/x/mongo/driver/operation/command.go +++ b/x/mongo/driver/operation/command.go @@ -14,7 +14,6 @@ import ( "go.mongodb.org/mongo-driver/event" "go.mongodb.org/mongo-driver/internal/logger" "go.mongodb.org/mongo-driver/mongo/description" - "go.mongodb.org/mongo-driver/mongo/readconcern" "go.mongodb.org/mongo-driver/mongo/readpref" "go.mongodb.org/mongo-driver/x/bsonx/bsoncore" "go.mongodb.org/mongo-driver/x/mongo/driver" @@ -23,41 +22,53 @@ import ( // Command is used to run a generic operation. type Command struct { - command bsoncore.Document - readConcern *readconcern.ReadConcern - database string - deployment driver.Deployment - selector description.ServerSelector - readPreference *readpref.ReadPref - clock *session.ClusterClock - session *session.Client - monitor *event.CommandMonitor - resultResponse bsoncore.Document - resultCursor *driver.BatchCursor - crypt driver.Crypt - serverAPI *driver.ServerAPIOptions - createCursor bool - cursorOpts driver.CursorOptions - timeout *time.Duration - logger *logger.Logger -} + // Command is the command document to send to the database. + Command bsoncore.Document -// NewCommand constructs and returns a new Command. Once the operation is executed, the result may only be accessed via -// the Result() function. -func NewCommand(command bsoncore.Document) *Command { - return &Command{ - command: command, - } -} + // Database is the database to run this operation against. + Database string -// NewCursorCommand constructs a new Command. Once the operation is executed, the server response will be used to -// construct a cursor, which can be accessed via the ResultCursor() function. -func NewCursorCommand(command bsoncore.Document, cursorOpts driver.CursorOptions) *Command { - return &Command{ - command: command, - cursorOpts: cursorOpts, - createCursor: true, - } + // Deployment is the deployment to use for this operation. + Deployment driver.Deployment + + // Selector is the server selector used to retrieve a server. + Selector description.ServerSelector + + // ReadPreference is the read preference used with this operation. + ReadPreference *readpref.ReadPref + + // Clock is the cluster clock for this operation. + Clock *session.ClusterClock + + // Session is the session for this operation. + Session *session.Client + + // Monitor is the monitor to use for APM events. + Monitor *event.CommandMonitor + + // Crypt is the Crypt object to use for automatic encryption and decryption. + Crypt driver.Crypt + + // ServerAPI is the server API version for this operation. + ServerAPI *driver.ServerAPIOptions + + // Timeout is the timeout for this operation. + Timeout *time.Duration + + // Logger is the logger for this operation. + Logger *logger.Logger + + // CreateCursor controls whether or not executing the command creates a + // cursor from the database response. It must be set to true to run commands + // that return a cursor. + CreateCursor bool + + // CursorOpts are the options to use when creating the cursor from the + // database response. + CursorOpts driver.CursorOptions + + resultResponse bsoncore.Document + resultCursor *driver.BatchCursor } // Result returns the result of executing this operation. @@ -67,7 +78,7 @@ func (c *Command) Result() bsoncore.Document { return c.resultResponse } // configured to create a cursor (i.e. it was created using NewCommand rather than NewCursorCommand), this function // will return nil and an error. func (c *Command) ResultCursor() (*driver.BatchCursor, error) { - if !c.createCursor { + if !c.CreateCursor { return nil, errors.New("command operation was not configured to create a cursor, but a result cursor was requested") } return c.resultCursor, nil @@ -75,160 +86,41 @@ func (c *Command) ResultCursor() (*driver.BatchCursor, error) { // Execute runs this operations and returns an error if the operation did not execute successfully. func (c *Command) Execute(ctx context.Context) error { - if c.deployment == nil { + if c.Deployment == nil { return errors.New("the Command operation must have a Deployment set before Execute can be called") } // TODO(GODRIVER-2649): Actually pass readConcern to underlying driver.Operation. return driver.Operation{ CommandFn: func(dst []byte, desc description.SelectedServer) ([]byte, error) { - return append(dst, c.command[4:len(c.command)-1]...), nil + return append(dst, c.Command[4:len(c.Command)-1]...), nil }, ProcessResponseFn: func(info driver.ResponseInfo) error { c.resultResponse = info.ServerResponse - if c.createCursor { + if c.CreateCursor { cursorRes, err := driver.NewCursorResponse(info) if err != nil { return err } - c.resultCursor, err = driver.NewBatchCursor(cursorRes, c.session, c.clock, c.cursorOpts) + c.resultCursor, err = driver.NewBatchCursor(cursorRes, c.Session, c.Clock, c.CursorOpts) return err } return nil }, - Client: c.session, - Clock: c.clock, - CommandMonitor: c.monitor, - Database: c.database, - Deployment: c.deployment, - ReadPreference: c.readPreference, - Selector: c.selector, - Crypt: c.crypt, - ServerAPI: c.serverAPI, - Timeout: c.timeout, - Logger: c.logger, + // TODO(GODRIVER-2649): Pass read concern to the operation. + Client: c.Session, + Clock: c.Clock, + CommandMonitor: c.Monitor, + Database: c.Database, + Deployment: c.Deployment, + ReadPreference: c.ReadPreference, + Selector: c.Selector, + Crypt: c.Crypt, + ServerAPI: c.ServerAPI, + Timeout: c.Timeout, + Logger: c.Logger, }.Execute(ctx) } - -// Session sets the session for this operation. -func (c *Command) Session(session *session.Client) *Command { - if c == nil { - c = new(Command) - } - - c.session = session - return c -} - -// ClusterClock sets the cluster clock for this operation. -func (c *Command) ClusterClock(clock *session.ClusterClock) *Command { - if c == nil { - c = new(Command) - } - - c.clock = clock - return c -} - -// CommandMonitor sets the monitor to use for APM events. -func (c *Command) CommandMonitor(monitor *event.CommandMonitor) *Command { - if c == nil { - c = new(Command) - } - - c.monitor = monitor - return c -} - -// Database sets the database to run this operation against. -func (c *Command) Database(database string) *Command { - if c == nil { - c = new(Command) - } - - c.database = database - return c -} - -// Deployment sets the deployment to use for this operation. -func (c *Command) Deployment(deployment driver.Deployment) *Command { - if c == nil { - c = new(Command) - } - - c.deployment = deployment - return c -} - -// ReadConcern specifies the read concern for this operation. -func (c *Command) ReadConcern(readConcern *readconcern.ReadConcern) *Command { - if c == nil { - c = new(Command) - } - - c.readConcern = readConcern - return c -} - -// ReadPreference set the read preference used with this operation. -func (c *Command) ReadPreference(readPreference *readpref.ReadPref) *Command { - if c == nil { - c = new(Command) - } - - c.readPreference = readPreference - return c -} - -// ServerSelector sets the selector used to retrieve a server. -func (c *Command) ServerSelector(selector description.ServerSelector) *Command { - if c == nil { - c = new(Command) - } - - c.selector = selector - return c -} - -// Crypt sets the Crypt object to use for automatic encryption and decryption. -func (c *Command) Crypt(crypt driver.Crypt) *Command { - if c == nil { - c = new(Command) - } - - c.crypt = crypt - return c -} - -// ServerAPI sets the server API version for this operation. -func (c *Command) ServerAPI(serverAPI *driver.ServerAPIOptions) *Command { - if c == nil { - c = new(Command) - } - - c.serverAPI = serverAPI - return c -} - -// Timeout sets the timeout for this operation. -func (c *Command) Timeout(timeout *time.Duration) *Command { - if c == nil { - c = new(Command) - } - - c.timeout = timeout - return c -} - -// Logger sets the logger for this operation. -func (c *Command) Logger(logger *logger.Logger) *Command { - if c == nil { - c = new(Command) - } - - c.logger = logger - return c -}