Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GODRIVER-3434 Revert builder-lister pattern for client options #1899

Merged
merged 15 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading