Skip to content

Commit

Permalink
GODRIVER-3434 Revert builder-lister pattern for client options (#1899)
Browse files Browse the repository at this point in the history
Co-authored-by: Qingyang Hu <103950869+qingyang-hu@users.noreply.github.com>
  • Loading branch information
prestonvasquez and qingyang-hu authored Dec 5, 2024
1 parent 7ea8947 commit 17a547f
Show file tree
Hide file tree
Showing 34 changed files with 549 additions and 1,185 deletions.
10 changes: 2 additions & 8 deletions internal/cmd/testatlas/atlas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (

"go.mongodb.org/mongo-driver/v2/bson"
"go.mongodb.org/mongo-driver/v2/internal/handshake"
"go.mongodb.org/mongo-driver/v2/internal/mongoutil"
"go.mongodb.org/mongo-driver/v2/mongo"
"go.mongodb.org/mongo-driver/v2/mongo/options"
)
Expand Down Expand Up @@ -46,12 +45,7 @@ func TestAtlas(t *testing.T) {
t.Fatalf("error running test with TLS at index %d: %v", idx, err)
}

args, err := mongoutil.NewOptions[options.ClientOptions](clientOpts)
if err != nil {
panic(fmt.Sprintf("failed to construct args from options: %v", err))
}

tlsConfigSkipVerify := args.TLSConfig
tlsConfigSkipVerify := clientOpts.TLSConfig
tlsConfigSkipVerify.InsecureSkipVerify = true

// Run the connectivity test with InsecureSkipVerify to ensure SNI is done correctly even if verification is
Expand All @@ -66,7 +60,7 @@ func TestAtlas(t *testing.T) {
t.Logf("Finished!")
}

func runTest(ctx context.Context, clientOpts *options.ClientOptionsBuilder) error {
func runTest(ctx context.Context, clientOpts *options.ClientOptions) error {
client, err := mongo.Connect(clientOpts)
if err != nil {
return fmt.Errorf("Connect error: %w", err)
Expand Down
8 changes: 4 additions & 4 deletions internal/integration/client_side_encryption_prose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ func TestClientSideEncryptionProse(t *testing.T) {
tlsConfig["kmip"] = kmipConfig
}

getBaseAutoEncryptionOpts := func() *options.AutoEncryptionOptionsBuilder {
getBaseAutoEncryptionOpts := func() *options.AutoEncryptionOptions {
return options.AutoEncryption().
SetKmsProviders(fullKmsProvidersMap).
SetKeyVaultNamespace(kvNamespace).
Expand All @@ -537,7 +537,7 @@ func TestClientSideEncryptionProse(t *testing.T) {

testCases := []struct {
name string
aeo *options.AutoEncryptionOptionsBuilder
aeo *options.AutoEncryptionOptions
schema bson.Raw // the schema to create the collection. if nil, the collection won't be explicitly created
}{
{"remote schema", getBaseAutoEncryptionOpts(), corpusSchema},
Expand Down Expand Up @@ -3014,7 +3014,7 @@ type cseProseTest struct {
cseStarted []*event.CommandStartedEvent
}

func setup(mt *mtest.T, aeo *options.AutoEncryptionOptionsBuilder, kvClientOpts options.Lister[options.ClientOptions],
func setup(mt *mtest.T, aeo *options.AutoEncryptionOptions, kvClientOpts *options.ClientOptions,
ceo options.Lister[options.ClientEncryptionOptions]) *cseProseTest {
mt.Helper()
var cpt cseProseTest
Expand Down Expand Up @@ -3093,7 +3093,7 @@ func rawValueToCoreValue(rv bson.RawValue) bsoncore.Value {

type deadlockTest struct {
clientTest *mongo.Client
clientKeyVaultOpts *options.ClientOptionsBuilder
clientKeyVaultOpts *options.ClientOptions
clientKeyVaultEvents []startedEvent
clientEncryption *mongo.ClientEncryption
ciphertext bson.Binary
Expand Down
12 changes: 2 additions & 10 deletions internal/integration/client_side_encryption_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,11 +356,7 @@ func TestClientSideEncryptionCustomCrypt(t *testing.T) {
ApplyURI(mtest.ClusterURI()).
SetAutoEncryptionOptions(aeOpts)
cc := &customCrypt{}
clientOpts.Opts = append(clientOpts.Opts, func(args *options.ClientOptions) error {
args.Crypt = cc

return nil
})
clientOpts.Crypt = cc
integtest.AddTestServerAPIVersion(clientOpts)

client, err := mongo.Connect(clientOpts)
Expand Down Expand Up @@ -683,11 +679,7 @@ func TestFLEIndexView(t *testing.T) {
SetReadPreference(mtest.PrimaryRp)

cc := &customCrypt{}
opts.Opts = append(opts.Opts, func(args *options.ClientOptions) error {
args.Crypt = cc

return nil
})
opts.Crypt = cc

integtest.AddTestServerAPIVersion(opts)

Expand Down
2 changes: 1 addition & 1 deletion internal/integration/data_lake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func TestAtlasDataLake(t *testing.T) {
})
}

func getBaseClientOptions(mt *mtest.T) *options.ClientOptionsBuilder {
func getBaseClientOptions(mt *mtest.T) *options.ClientOptions {
mt.Helper()

hosts, err := mongoutil.HostsFromURI(mtest.ClusterURI())
Expand Down
2 changes: 1 addition & 1 deletion internal/integration/handshake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func TestHandshakeProse(t *testing.T) {
for _, test := range []struct {
name string
env map[string]string
opts *options.ClientOptionsBuilder
opts *options.ClientOptions
want bson.D
}{
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func runSeedlistDiscoveryDirectory(mt *mtest.T, subdirectory string) {
}

// runSeedlistDiscoveryPingTest will create a new connection using the test URI and attempt to "ping" the server.
func runSeedlistDiscoveryPingTest(mt *mtest.T, clientOpts *options.ClientOptionsBuilder) {
func runSeedlistDiscoveryPingTest(mt *mtest.T, clientOpts *options.ClientOptions) {
ctx := context.Background()

client, err := mongo.Connect(clientOpts)
Expand Down
4 changes: 2 additions & 2 deletions internal/integration/json_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func jsonFilesInDir(t testing.TB, dir string) []string {
}

// create client options from a map
func createClientOptions(t testing.TB, opts bson.Raw) *options.ClientOptionsBuilder {
func createClientOptions(t testing.TB, opts bson.Raw) *options.ClientOptions {
t.Helper()

clientOpts := options.Client()
Expand Down Expand Up @@ -125,7 +125,7 @@ func createClientOptions(t testing.TB, opts bson.Raw) *options.ClientOptionsBuil
return clientOpts
}

func createAutoEncryptionOptions(t testing.TB, opts bson.Raw) *options.AutoEncryptionOptionsBuilder {
func createAutoEncryptionOptions(t testing.TB, opts bson.Raw) *options.AutoEncryptionOptions {
t.Helper()

aeo := options.AutoEncryption()
Expand Down
30 changes: 12 additions & 18 deletions internal/integration/mtest/mongotest.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ type T struct {

// options copied to sub-tests
clientType ClientType
clientOpts *options.ClientOptionsBuilder
clientOpts *options.ClientOptions
collOpts *options.CollectionOptionsBuilder
shareClient *bool

Expand Down Expand Up @@ -359,7 +359,7 @@ func (t *T) ClearEvents() {
// If t.Coll is not-nil, it will be reset to use the new client. Should only be called if the existing client is
// not nil. This will Disconnect the existing client but will not drop existing collections. To do so, ClearCollections
// must be called before calling ResetClient.
func (t *T) ResetClient(opts *options.ClientOptionsBuilder) {
func (t *T) ResetClient(opts *options.ClientOptions) {
if opts != nil {
t.clientOpts = opts
}
Expand Down Expand Up @@ -592,18 +592,13 @@ func (t *T) createTestClient() {
clientOpts = options.Client().SetWriteConcern(MajorityWc).SetReadPreference(PrimaryRp)
}

args, err := mongoutil.NewOptions[options.ClientOptions](clientOpts)
if err != nil {
t.Fatalf("failed to construct options from builder: %v", err)
}

// set ServerAPIOptions to latest version if required
if args.Deployment == nil && t.clientType != Mock && args.ServerAPIOptions == nil && testContext.requireAPIVersion {
if clientOpts.Deployment == nil && t.clientType != Mock && clientOpts.ServerAPIOptions == nil && testContext.requireAPIVersion {
clientOpts.SetServerAPIOptions(options.ServerAPI(driver.TestServerAPIVersion))
}

// Setup command monitor
var customMonitor = args.Monitor
var customMonitor = clientOpts.Monitor
clientOpts.SetMonitor(&event.CommandMonitor{
Started: func(ctx context.Context, cse *event.CommandStartedEvent) {
if customMonitor != nil && customMonitor.Started != nil {
Expand Down Expand Up @@ -631,8 +626,8 @@ func (t *T) createTestClient() {
},
})
// only specify connection pool monitor if no deployment is given
if args.Deployment == nil {
previousPoolMonitor := args.PoolMonitor
if clientOpts.Deployment == nil {
previousPoolMonitor := clientOpts.PoolMonitor

clientOpts.SetPoolMonitor(&event.PoolMonitor{
Event: func(evt *event.PoolEvent) {
Expand All @@ -650,6 +645,7 @@ func (t *T) createTestClient() {
})
}

var err error
switch t.clientType {
case Pinned:
// pin to first mongos
Expand All @@ -658,15 +654,13 @@ func (t *T) createTestClient() {
t.Client, err = mongo.Connect(uriOpts, clientOpts)
case Mock:
// clear pool monitor to avoid configuration error
args, _ = mongoutil.NewOptions[options.ClientOptions](clientOpts)

args.PoolMonitor = nil
clientOpts.PoolMonitor = nil

t.mockDeployment = drivertest.NewMockDeployment()
args.Deployment = t.mockDeployment
clientOpts.Deployment = t.mockDeployment

opts := mongoutil.NewOptionsLister(args, nil)
t.Client, err = mongo.Connect(opts)
t.Client, err = mongo.Connect(clientOpts)
case Proxy:
t.proxyDialer = newProxyDialer()
clientOpts.SetDialer(t.proxyDialer)
Expand All @@ -676,8 +670,8 @@ func (t *T) createTestClient() {
case Default:
// Use a different set of options to specify the URI because clientOpts may already have a URI or host seedlist
// specified.
var uriOpts *options.ClientOptionsBuilder
if args.Deployment == nil {
var uriOpts *options.ClientOptions
if clientOpts.Deployment == nil {
// Only specify URI if the deployment is not set to avoid setting topology/server options along with the
// deployment.
uriOpts = options.Client().ApplyURI(testContext.connString.Original)
Expand Down
2 changes: 1 addition & 1 deletion internal/integration/mtest/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (op *Options) CollectionOptions(opts *options.CollectionOptionsBuilder) *Op
}

// ClientOptions sets the options to use when creating a client for a test.
func (op *Options) ClientOptions(opts *options.ClientOptionsBuilder) *Options {
func (op *Options) ClientOptions(opts *options.ClientOptions) *Options {
op.optFuncs = append(op.optFuncs, func(t *T) {
t.clientOpts = opts
})
Expand Down
12 changes: 3 additions & 9 deletions internal/integration/mtest/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (

"go.mongodb.org/mongo-driver/v2/bson"
"go.mongodb.org/mongo-driver/v2/internal/integtest"
"go.mongodb.org/mongo-driver/v2/internal/mongoutil"
"go.mongodb.org/mongo-driver/v2/mongo"
"go.mongodb.org/mongo-driver/v2/mongo/options"
"go.mongodb.org/mongo-driver/v2/mongo/readpref"
Expand Down Expand Up @@ -58,20 +57,15 @@ var testContext struct {
serverless bool
}

func setupClient(opts *options.ClientOptionsBuilder) (*mongo.Client, error) {
args, err := mongoutil.NewOptions[options.ClientOptions](opts)
if err != nil {
return nil, fmt.Errorf("failed to construct options from builder: %w", err)
}

func setupClient(opts *options.ClientOptions) (*mongo.Client, error) {
wcMajority := writeconcern.Majority()
// set ServerAPIOptions to latest version if required
if args.ServerAPIOptions == nil && testContext.requireAPIVersion {
if opts.ServerAPIOptions == nil && testContext.requireAPIVersion {
opts.SetServerAPIOptions(options.ServerAPI(driver.TestServerAPIVersion))
}
// for sharded clusters, pin to one host. Due to how the cache is implemented on 4.0 and 4.2, behavior
// can be inconsistent when multiple mongoses are used
return mongo.Connect(opts.SetWriteConcern(wcMajority).SetHosts(args.Hosts[:1]))
return mongo.Connect(opts.SetWriteConcern(wcMajority).SetHosts(opts.Hosts[:1]))
}

// Setup initializes the current testing context.
Expand Down
2 changes: 1 addition & 1 deletion internal/integration/sdam_error_handling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

func TestSDAMErrorHandling(t *testing.T) {
mt := mtest.New(t, noClientOpts)
baseClientOpts := func() *options.ClientOptionsBuilder {
baseClientOpts := func() *options.ClientOptions {
return options.Client().
ApplyURI(mtest.ClusterURI()).
SetRetryWrites(false).
Expand Down
14 changes: 4 additions & 10 deletions internal/integration/unified/client_entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"go.mongodb.org/mongo-driver/v2/internal/integration/mtest"
"go.mongodb.org/mongo-driver/v2/internal/integtest"
"go.mongodb.org/mongo-driver/v2/internal/logger"
"go.mongodb.org/mongo-driver/v2/internal/mongoutil"
"go.mongodb.org/mongo-driver/v2/mongo"
"go.mongodb.org/mongo-driver/v2/mongo/options"
"go.mongodb.org/mongo-driver/v2/mongo/readconcern"
Expand Down Expand Up @@ -184,15 +183,10 @@ func newClientEntity(ctx context.Context, em *EntityMap, entityOptions *entityOp
}
}
if entityOptions.ServerAPIOptions != nil {
args, err := mongoutil.NewOptions[options.ServerAPIOptions](entityOptions.ServerAPIOptions)
if err != nil {
return nil, fmt.Errorf("failed to construct options from builder: %w", err)
}

if err := args.ServerAPIVersion.Validate(); err != nil {
if err := entityOptions.ServerAPIOptions.ServerAPIVersion.Validate(); err != nil {
return nil, err
}
clientOpts.SetServerAPIOptions(entityOptions.ServerAPIOptions.ServerAPIOptionsBuilder)
clientOpts.SetServerAPIOptions(entityOptions.ServerAPIOptions.ServerAPIOptions)
} else {
integtest.AddTestServerAPIVersion(clientOpts)
}
Expand Down Expand Up @@ -589,7 +583,7 @@ func (c *clientEntity) getRecordEvents() bool {
return c.recordEvents.Load().(bool)
}

func setClientOptionsFromURIOptions(clientOpts *options.ClientOptionsBuilder, uriOpts bson.M) error {
func setClientOptionsFromURIOptions(clientOpts *options.ClientOptions, uriOpts bson.M) error {
// A write concern can be constructed across multiple URI options (e.g. "w", "j", and "wTimeoutMS") so we declare an
// empty writeConcern instance here that can be populated in the loop below.
var wc writeConcern
Expand Down Expand Up @@ -654,7 +648,7 @@ func setClientOptionsFromURIOptions(clientOpts *options.ClientOptionsBuilder, ur
return nil
}

func evaluateUseMultipleMongoses(clientOpts *options.ClientOptionsBuilder, useMultipleMongoses bool) error {
func evaluateUseMultipleMongoses(clientOpts *options.ClientOptions, useMultipleMongoses bool) error {
hosts := mtest.ClusterConnString().Hosts

if !useMultipleMongoses {
Expand Down
4 changes: 2 additions & 2 deletions internal/integration/unified/server_api_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
// serverAPIOptions is a wrapper for *options.ServerAPIOptions. This type implements the bson.Unmarshaler interface
// to convert BSON documents to a serverAPIOptions instance.
type serverAPIOptions struct {
*options.ServerAPIOptionsBuilder
*options.ServerAPIOptions
}

type serverAPIVersion = options.ServerAPIVersion
Expand All @@ -37,7 +37,7 @@ func (s *serverAPIOptions) UnmarshalBSON(data []byte) error {
return fmt.Errorf("unrecognized fields for serverAPIOptions: %v", mapKeys(temp.Extra))
}

s.ServerAPIOptionsBuilder = options.ServerAPI(temp.ServerAPIVersion)
s.ServerAPIOptions = options.ServerAPI(temp.ServerAPIVersion)
if temp.DeprecationErrors != nil {
s.SetDeprecationErrors(*temp.DeprecationErrors)
}
Expand Down
Loading

0 comments on commit 17a547f

Please sign in to comment.