From 0ae490d045eff100a0c3d080c3f805fde53dec20 Mon Sep 17 00:00:00 2001 From: Preston Vasquez Date: Fri, 19 Jul 2024 17:19:08 -0600 Subject: [PATCH 1/4] GODRIVER-2689 Simplify the readpref API --- event/description.go | 4 +- internal/driverutil/description.go | 4 +- .../integration/unified/common_options.go | 20 +- internal/serverselector/server_selector.go | 29 ++- .../serverselector/server_selector_test.go | 185 ++++++++++-------- mongo/client_test.go | 24 +-- mongo/database.go | 2 +- mongo/options/clientoptions.go | 26 ++- mongo/options/clientoptions_test.go | 27 +-- mongo/readpref/mode.go | 14 -- mongo/readpref/options.go | 83 -------- mongo/readpref/options_example_test.go | 60 ------ mongo/readpref/readpref.go | 103 +++------- mongo/readpref/readpref_test.go | 132 ++----------- {tag => mongo/readpref}/tag.go | 47 +++-- {tag => mongo/readpref}/tag_test.go | 20 +- x/mongo/driver/description/server.go | 4 +- x/mongo/driver/operation.go | 22 +-- x/mongo/driver/operation_test.go | 63 ++++-- 19 files changed, 321 insertions(+), 548 deletions(-) delete mode 100644 mongo/readpref/options.go delete mode 100644 mongo/readpref/options_example_test.go rename {tag => mongo/readpref}/tag.go (66%) rename {tag => mongo/readpref}/tag_test.go (81%) diff --git a/event/description.go b/event/description.go index 682d61c1c9..1640ae8aaa 100644 --- a/event/description.go +++ b/event/description.go @@ -11,7 +11,7 @@ import ( "go.mongodb.org/mongo-driver/bson" "go.mongodb.org/mongo-driver/mongo/address" - "go.mongodb.org/mongo-driver/tag" + "go.mongodb.org/mongo-driver/mongo/readpref" ) // ServerDescription contains information about a node in a cluster. This is @@ -43,7 +43,7 @@ type ServerDescription struct { SessionTimeoutMinutes *int64 SetName string SetVersion uint32 - Tags tag.Set + Tags readpref.TagSet TopologyVersionProcessID bson.ObjectID TopologyVersionCounter int64 } diff --git a/internal/driverutil/description.go b/internal/driverutil/description.go index 6c04ec0b0a..54eb5b4699 100644 --- a/internal/driverutil/description.go +++ b/internal/driverutil/description.go @@ -16,7 +16,7 @@ import ( "go.mongodb.org/mongo-driver/internal/handshake" "go.mongodb.org/mongo-driver/internal/ptrutil" "go.mongodb.org/mongo-driver/mongo/address" - "go.mongodb.org/mongo-driver/tag" + "go.mongodb.org/mongo-driver/mongo/readpref" "go.mongodb.org/mongo-driver/x/mongo/driver/description" ) @@ -421,7 +421,7 @@ func NewServerDescription(addr address.Address, response bson.Raw) description.S desc.LastError = err return desc } - desc.Tags = tag.NewTagSetFromMap(m) + desc.Tags = readpref.NewTagSetFromMap(m) case "topologyVersion": doc, ok := element.Value().DocumentOK() if !ok { diff --git a/internal/integration/unified/common_options.go b/internal/integration/unified/common_options.go index 2b78466a9b..1282f86c89 100644 --- a/internal/integration/unified/common_options.go +++ b/internal/integration/unified/common_options.go @@ -14,7 +14,6 @@ import ( "go.mongodb.org/mongo-driver/mongo/readconcern" "go.mongodb.org/mongo-driver/mongo/readpref" "go.mongodb.org/mongo-driver/mongo/writeconcern" - "go.mongodb.org/mongo-driver/tag" ) // This file defines helper types to convert BSON documents to ReadConcern, WriteConcern, and ReadPref objects. @@ -70,32 +69,35 @@ func (rp *ReadPreference) ToReadPrefOption() (*readpref.ReadPref, error) { return nil, fmt.Errorf("invalid read preference mode %q", rp.Mode) } - var rpOptions []readpref.Option + newReadPref := &readpref.ReadPref{Mode: mode} + if rp.TagSets != nil { // Each item in the TagSets slice is a document that represents one set. - sets := make([]tag.Set, 0, len(rp.TagSets)) + sets := make([]readpref.TagSet, 0, len(rp.TagSets)) for _, rawSet := range rp.TagSets { - parsed := make(tag.Set, 0, len(rawSet)) + parsed := make(readpref.TagSet, 0, len(rawSet)) for k, v := range rawSet { - parsed = append(parsed, tag.Tag{Name: k, Value: v}) + parsed = append(parsed, readpref.Tag{Name: k, Value: v}) } sets = append(sets, parsed) } - rpOptions = append(rpOptions, readpref.WithTagSets(sets...)) + newReadPref.TagSets = sets } if rp.MaxStalenessSeconds != nil { maxStaleness := time.Duration(*rp.MaxStalenessSeconds) * time.Second - rpOptions = append(rpOptions, readpref.WithMaxStaleness(maxStaleness)) + newReadPref.MaxStaleness = &maxStaleness + } if rp.Hedge != nil { if len(rp.Hedge) > 1 { return nil, fmt.Errorf("invalid read preference hedge document: length cannot be greater than 1") } if enabled, ok := rp.Hedge["enabled"]; ok { - rpOptions = append(rpOptions, readpref.WithHedgeEnabled(enabled.(bool))) + hedgeEnabled := enabled.(bool) + newReadPref.HedgeEnabled = &hedgeEnabled } } - return readpref.New(mode, rpOptions...) + return newReadPref, nil } diff --git a/internal/serverselector/server_selector.go b/internal/serverselector/server_selector.go index 4599b0f9d3..57fe1828f8 100644 --- a/internal/serverselector/server_selector.go +++ b/internal/serverselector/server_selector.go @@ -12,7 +12,6 @@ import ( "time" "go.mongodb.org/mongo-driver/mongo/readpref" - "go.mongodb.org/mongo-driver/tag" "go.mongodb.org/mongo-driver/x/mongo/driver/description" ) @@ -190,12 +189,12 @@ func (ssf Func) SelectServer( } func verifyMaxStaleness(rp *readpref.ReadPref, topo description.Topology) error { - maxStaleness, set := rp.MaxStaleness() - if !set { + maxStaleness := rp.MaxStaleness + if maxStaleness == nil { return nil } - if maxStaleness < 90*time.Second { + if *maxStaleness < 90*time.Second { return fmt.Errorf("max staleness (%s) must be greater than or equal to 90s", maxStaleness) } @@ -208,7 +207,7 @@ func verifyMaxStaleness(rp *readpref.ReadPref, topo description.Topology) error s := topo.Servers[0] idleWritePeriod := 10 * time.Second - if maxStaleness < s.HeartbeatInterval+idleWritePeriod { + if *maxStaleness < s.HeartbeatInterval+idleWritePeriod { return fmt.Errorf( "max staleness (%s) must be greater than or equal to the heartbeat interval (%s) plus idle write period (%s)", maxStaleness, s.HeartbeatInterval, idleWritePeriod, @@ -242,7 +241,7 @@ func selectSecondaries(rp *readpref.ReadPref, candidates []description.Server) [ if len(secondaries) == 0 { return secondaries } - if maxStaleness, set := rp.MaxStaleness(); set { + if maxStaleness := rp.MaxStaleness; maxStaleness != nil { primaries := selectByKind(candidates, description.ServerKindRSPrimary) if len(primaries) == 0 { baseTime := secondaries[0].LastWriteTime @@ -255,7 +254,7 @@ func selectSecondaries(rp *readpref.ReadPref, candidates []description.Server) [ var selected []description.Server for _, secondary := range secondaries { estimatedStaleness := baseTime.Sub(secondary.LastWriteTime) + secondary.HeartbeatInterval - if estimatedStaleness <= maxStaleness { + if estimatedStaleness <= *maxStaleness { selected = append(selected, secondary) } } @@ -269,7 +268,7 @@ func selectSecondaries(rp *readpref.ReadPref, candidates []description.Server) [ for _, secondary := range secondaries { estimatedStaleness := secondary.LastUpdateTime.Sub(secondary.LastWriteTime) - primary.LastUpdateTime.Sub(primary.LastWriteTime) + secondary.HeartbeatInterval - if estimatedStaleness <= maxStaleness { + if estimatedStaleness <= *maxStaleness { selected = append(selected, secondary) } } @@ -279,7 +278,7 @@ func selectSecondaries(rp *readpref.ReadPref, candidates []description.Server) [ return secondaries } -func selectByTagSet(candidates []description.Server, tagSets []tag.Set) []description.Server { +func selectByTagSet(candidates []description.Server, tagSets []readpref.TagSet) []description.Server { if len(tagSets) == 0 { return candidates } @@ -327,7 +326,7 @@ func selectForReplicaSet( } } - switch rp.Mode() { + switch rp.Mode { case readpref.PrimaryMode: return selectByKind(candidates, description.ServerKindRSPrimary), nil case readpref.PrimaryPreferredMode: @@ -335,25 +334,25 @@ func selectForReplicaSet( if len(selected) == 0 { selected = selectSecondaries(rp, candidates) - return selectByTagSet(selected, rp.TagSets()), nil + return selectByTagSet(selected, rp.TagSets), nil } return selected, nil case readpref.SecondaryPreferredMode: selected := selectSecondaries(rp, candidates) - selected = selectByTagSet(selected, rp.TagSets()) + selected = selectByTagSet(selected, rp.TagSets) if len(selected) > 0 { return selected, nil } return selectByKind(candidates, description.ServerKindRSPrimary), nil case readpref.SecondaryMode: selected := selectSecondaries(rp, candidates) - return selectByTagSet(selected, rp.TagSets()), nil + return selectByTagSet(selected, rp.TagSets), nil case readpref.NearestMode: selected := selectByKind(candidates, description.ServerKindRSPrimary) selected = append(selected, selectSecondaries(rp, candidates)...) - return selectByTagSet(selected, rp.TagSets()), nil + return selectByTagSet(selected, rp.TagSets), nil } - return nil, fmt.Errorf("unsupported mode: %d", rp.Mode()) + return nil, fmt.Errorf("unsupported mode: %d", rp.Mode) } diff --git a/internal/serverselector/server_selector_test.go b/internal/serverselector/server_selector_test.go index a8f212aeca..8c119cf49f 100644 --- a/internal/serverselector/server_selector_test.go +++ b/internal/serverselector/server_selector_test.go @@ -21,7 +21,6 @@ import ( "go.mongodb.org/mongo-driver/internal/spectest" "go.mongodb.org/mongo-driver/mongo/address" "go.mongodb.org/mongo-driver/mongo/readpref" - "go.mongodb.org/mongo-driver/tag" "go.mongodb.org/mongo-driver/x/mongo/driver/description" ) @@ -115,7 +114,7 @@ func topologyKindFromString(t *testing.T, s string) description.TopologyKind { return description.Unknown } -func anyTagsInSets(sets []tag.Set) bool { +func anyTagsInSets(sets []readpref.TagSet) bool { for _, set := range sets { if len(set) > 0 { return true @@ -219,7 +218,7 @@ func selectServers(t *testing.T, test *testCase) error { } if serverDescription.Tags != nil { - server.Tags = tag.NewTagSetFromMap(serverDescription.Tags) + server.Tags = readpref.NewTagSetFromMap(serverDescription.Tags) } if test.ReadPreference.MaxStaleness != nil && server.WireVersion == nil { @@ -243,21 +242,16 @@ func selectServers(t *testing.T, test *testCase) error { return err } - options := make([]readpref.Option, 0, 1) + rp := &readpref.ReadPref{Mode: readprefMode} - tagSets := tag.NewTagSetsFromMaps(test.ReadPreference.TagSets) + tagSets := readpref.NewTagSetsFromMaps(test.ReadPreference.TagSets) if anyTagsInSets(tagSets) { - options = append(options, readpref.WithTagSets(tagSets...)) + rp.TagSets = tagSets } if test.ReadPreference.MaxStaleness != nil { s := time.Duration(*test.ReadPreference.MaxStaleness) * time.Second - options = append(options, readpref.WithMaxStaleness(s)) - } - - rp, err := readpref.New(readprefMode, options...) - if err != nil { - return err + rp.MaxStaleness = &s } var selector description.ServerSelector @@ -496,7 +490,7 @@ var readPrefTestPrimary = description.Server{ LastWriteTime: time.Date(2017, 2, 11, 14, 0, 0, 0, time.UTC), LastUpdateTime: time.Date(2017, 2, 11, 14, 0, 2, 0, time.UTC), Kind: description.ServerKindRSPrimary, - Tags: tag.Set{tag.Tag{Name: "a", Value: "1"}}, + Tags: readpref.TagSet{readpref.Tag{Name: "a", Value: "1"}}, WireVersion: &description.VersionRange{Min: 6, Max: 21}, } var readPrefTestSecondary1 = description.Server{ @@ -505,7 +499,7 @@ var readPrefTestSecondary1 = description.Server{ LastWriteTime: time.Date(2017, 2, 11, 13, 58, 0, 0, time.UTC), LastUpdateTime: time.Date(2017, 2, 11, 14, 0, 2, 0, time.UTC), Kind: description.ServerKindRSSecondary, - Tags: tag.Set{tag.Tag{Name: "a", Value: "1"}}, + Tags: readpref.TagSet{readpref.Tag{Name: "a", Value: "1"}}, WireVersion: &description.VersionRange{Min: 6, Max: 21}, } var readPrefTestSecondary2 = description.Server{ @@ -514,7 +508,7 @@ var readPrefTestSecondary2 = description.Server{ LastWriteTime: time.Date(2017, 2, 11, 14, 0, 0, 0, time.UTC), LastUpdateTime: time.Date(2017, 2, 11, 14, 0, 2, 0, time.UTC), Kind: description.ServerKindRSSecondary, - Tags: tag.Set{tag.Tag{Name: "a", Value: "2"}}, + Tags: readpref.TagSet{readpref.Tag{Name: "a", Value: "2"}}, WireVersion: &description.VersionRange{Min: 6, Max: 21}, } var readPrefTestTopology = description.Topology{ @@ -768,9 +762,12 @@ func TestSelector_PrimaryPreferred(t *testing.T) { func TestSelector_PrimaryPreferred_ignores_tags(t *testing.T) { t.Parallel() - subject := readpref.PrimaryPreferred( - readpref.WithTags("a", "2"), - ) + subject := readpref.PrimaryPreferred() + + tagSet, err := readpref.NewTagSet("a", "2") + assert.NoError(t, err) + + subject.TagSets = []readpref.TagSet{tagSet} result, err := (&ReadPref{ReadPref: subject}). SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -796,9 +793,12 @@ func TestSelector_PrimaryPreferred_with_no_primary(t *testing.T) { func TestSelector_PrimaryPreferred_with_no_primary_and_tags(t *testing.T) { t.Parallel() - subject := readpref.PrimaryPreferred( - readpref.WithTags("a", "2"), - ) + subject := readpref.PrimaryPreferred() + + tagSet, err := readpref.NewTagSet("a", "2") + assert.NoError(t, err) + + subject.TagSets = []readpref.TagSet{tagSet} result, err := (&ReadPref{ReadPref: subject}). SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -811,9 +811,10 @@ func TestSelector_PrimaryPreferred_with_no_primary_and_tags(t *testing.T) { func TestSelector_PrimaryPreferred_with_maxStaleness(t *testing.T) { t.Parallel() - subject := readpref.PrimaryPreferred( - readpref.WithMaxStaleness(time.Duration(90) * time.Second), - ) + subject := readpref.PrimaryPreferred() + + maxStaleness := 90 * time.Second + subject.MaxStaleness = &maxStaleness result, err := (&ReadPref{ReadPref: subject}). SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -826,9 +827,10 @@ func TestSelector_PrimaryPreferred_with_maxStaleness(t *testing.T) { func TestSelector_PrimaryPreferred_with_maxStaleness_and_no_primary(t *testing.T) { t.Parallel() - subject := readpref.PrimaryPreferred( - readpref.WithMaxStaleness(time.Duration(90) * time.Second), - ) + subject := readpref.PrimaryPreferred() + + maxStaleness := 90 * time.Second + subject.MaxStaleness = &maxStaleness result, err := (&ReadPref{ReadPref: subject}). SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -853,9 +855,12 @@ func TestSelector_SecondaryPreferred(t *testing.T) { func TestSelector_SecondaryPreferred_with_tags(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred( - readpref.WithTags("a", "2"), - ) + subject := readpref.SecondaryPreferred() + + tagSet, err := readpref.NewTagSet("a", "2") + assert.NoError(t, err) + + subject.TagSets = []readpref.TagSet{tagSet} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -867,9 +872,12 @@ func TestSelector_SecondaryPreferred_with_tags(t *testing.T) { func TestSelector_SecondaryPreferred_with_tags_that_do_not_match(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred( - readpref.WithTags("a", "3"), - ) + subject := readpref.SecondaryPreferred() + + tagSet, err := readpref.NewTagSet("a", "3") + assert.NoError(t, err) + + subject.TagSets = []readpref.TagSet{tagSet} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -881,9 +889,12 @@ func TestSelector_SecondaryPreferred_with_tags_that_do_not_match(t *testing.T) { func TestSelector_SecondaryPreferred_with_tags_that_do_not_match_and_no_primary(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred( - readpref.WithTags("a", "3"), - ) + subject := readpref.SecondaryPreferred() + + tagSet, err := readpref.NewTagSet("a", "3") + assert.NoError(t, err) + + subject.TagSets = []readpref.TagSet{tagSet} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -917,9 +928,10 @@ func TestSelector_SecondaryPreferred_with_no_secondaries_or_primary(t *testing.T func TestSelector_SecondaryPreferred_with_maxStaleness(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred( - readpref.WithMaxStaleness(time.Duration(90) * time.Second), - ) + subject := readpref.SecondaryPreferred() + + maxStaleness := 90 * time.Second + subject.MaxStaleness = &maxStaleness result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -931,9 +943,10 @@ func TestSelector_SecondaryPreferred_with_maxStaleness(t *testing.T) { func TestSelector_SecondaryPreferred_with_maxStaleness_and_no_primary(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred( - readpref.WithMaxStaleness(time.Duration(90) * time.Second), - ) + subject := readpref.SecondaryPreferred() + + maxStaleness := 90 * time.Second + subject.MaxStaleness = &maxStaleness result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -957,9 +970,10 @@ func TestSelector_Secondary(t *testing.T) { func TestSelector_Secondary_with_tags(t *testing.T) { t.Parallel() - subject := readpref.Secondary( - readpref.WithTags("a", "2"), - ) + subject := readpref.SecondaryPreferred() + + maxStaleness := 90 * time.Second + subject.MaxStaleness = &maxStaleness result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -991,13 +1005,13 @@ func TestSelector_Secondary_with_empty_tag_set(t *testing.T) { Servers: []description.Server{primaryNoTags, firstSecondaryNoTags, secondSecondaryNoTags}, } - nonMatchingSet := tag.Set{ + nonMatchingSet := readpref.TagSet{ {Name: "foo", Value: "bar"}, } - emptyTagSet := tag.Set{} - rp := readpref.Secondary( - readpref.WithTagSets(nonMatchingSet, emptyTagSet), - ) + emptyTagSet := readpref.TagSet{} + + rp := readpref.SecondaryPreferred() + rp.TagSets = []readpref.TagSet{nonMatchingSet, emptyTagSet} result, err := (&ReadPref{ReadPref: rp}).SelectServer(topologyNoTags, topologyNoTags.Servers) assert.Nil(t, err, "SelectServer error: %v", err) @@ -1008,9 +1022,12 @@ func TestSelector_Secondary_with_empty_tag_set(t *testing.T) { func TestSelector_Secondary_with_tags_that_do_not_match(t *testing.T) { t.Parallel() - subject := readpref.Secondary( - readpref.WithTags("a", "3"), - ) + subject := readpref.Secondary() + + tagSet, err := readpref.NewTagSet("a", "3") + assert.NoError(t, err) + + subject.TagSets = []readpref.TagSet{tagSet} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1032,9 +1049,10 @@ func TestSelector_Secondary_with_no_secondaries(t *testing.T) { func TestSelector_Secondary_with_maxStaleness(t *testing.T) { t.Parallel() - subject := readpref.Secondary( - readpref.WithMaxStaleness(time.Duration(90) * time.Second), - ) + subject := readpref.Secondary() + + maxStaleness := 90 * time.Second + subject.MaxStaleness = &maxStaleness result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1046,9 +1064,10 @@ func TestSelector_Secondary_with_maxStaleness(t *testing.T) { func TestSelector_Secondary_with_maxStaleness_and_no_primary(t *testing.T) { t.Parallel() - subject := readpref.Secondary( - readpref.WithMaxStaleness(time.Duration(90) * time.Second), - ) + subject := readpref.Secondary() + + maxStaleness := 90 * time.Second + subject.MaxStaleness = &maxStaleness result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -1072,9 +1091,12 @@ func TestSelector_Nearest(t *testing.T) { func TestSelector_Nearest_with_tags(t *testing.T) { t.Parallel() - subject := readpref.Nearest( - readpref.WithTags("a", "1"), - ) + subject := readpref.Nearest() + + tagSet, err := readpref.NewTagSet("a", "1") + assert.NoError(t, err) + + subject.TagSets = []readpref.TagSet{tagSet} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1086,9 +1108,12 @@ func TestSelector_Nearest_with_tags(t *testing.T) { func TestSelector_Nearest_with_tags_that_do_not_match(t *testing.T) { t.Parallel() - subject := readpref.Nearest( - readpref.WithTags("a", "3"), - ) + subject := readpref.Nearest() + + tagSet, err := readpref.NewTagSet("a", "3") + assert.NoError(t, err) + + subject.TagSets = []readpref.TagSet{tagSet} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1123,9 +1148,10 @@ func TestSelector_Nearest_with_no_secondaries(t *testing.T) { func TestSelector_Nearest_with_maxStaleness(t *testing.T) { t.Parallel() - subject := readpref.Nearest( - readpref.WithMaxStaleness(time.Duration(90) * time.Second), - ) + subject := readpref.Nearest() + + maxStaleness := 90 * time.Second + subject.MaxStaleness = &maxStaleness result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1137,9 +1163,10 @@ func TestSelector_Nearest_with_maxStaleness(t *testing.T) { func TestSelector_Nearest_with_maxStaleness_and_no_primary(t *testing.T) { t.Parallel() - subject := readpref.Nearest( - readpref.WithMaxStaleness(time.Duration(90) * time.Second), - ) + subject := readpref.Nearest() + + maxStaleness := 90 * time.Second + subject.MaxStaleness = &maxStaleness result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -1151,9 +1178,10 @@ func TestSelector_Nearest_with_maxStaleness_and_no_primary(t *testing.T) { func TestSelector_Max_staleness_is_less_than_90_seconds(t *testing.T) { t.Parallel() - subject := readpref.Nearest( - readpref.WithMaxStaleness(time.Duration(50) * time.Second), - ) + subject := readpref.Nearest() + + maxStaleness := 50 * time.Second + subject.MaxStaleness = &maxStaleness s := description.Server{ Addr: address.Address("localhost:27017"), @@ -1176,9 +1204,10 @@ func TestSelector_Max_staleness_is_less_than_90_seconds(t *testing.T) { func TestSelector_Max_staleness_is_too_low(t *testing.T) { t.Parallel() - subject := readpref.Nearest( - readpref.WithMaxStaleness(time.Duration(100) * time.Second), - ) + subject := readpref.Nearest() + + maxStaleness := 100 * time.Second + subject.MaxStaleness = &maxStaleness s := description.Server{ Addr: address.Address("localhost:27017"), @@ -1238,7 +1267,7 @@ func TestEqualServers(t *testing.T) { }, {"setName", description.Server{SetName: "foo"}, false}, {"setVersion", description.Server{SetVersion: 1}, false}, - {"tags", description.Server{Tags: tag.Set{tag.Tag{"foo", "bar"}}}, false}, + {"tags", description.Server{Tags: readpref.TagSet{readpref.Tag{"foo", "bar"}}}, false}, {"topologyVersion", description.Server{TopologyVersion: &description.TopologyVersion{bson.NewObjectID(), 0}}, false}, {"kind", description.Server{Kind: description.ServerKindStandalone}, false}, {"wireVersion", description.Server{WireVersion: &description.VersionRange{1, 2}}, false}, diff --git a/mongo/client_test.go b/mongo/client_test.go index 6e7607be91..44d7c13aec 100644 --- a/mongo/client_test.go +++ b/mongo/client_test.go @@ -18,11 +18,11 @@ import ( "go.mongodb.org/mongo-driver/event" "go.mongodb.org/mongo-driver/internal/assert" "go.mongodb.org/mongo-driver/internal/integtest" + "go.mongodb.org/mongo-driver/internal/require" "go.mongodb.org/mongo-driver/mongo/options" "go.mongodb.org/mongo-driver/mongo/readconcern" "go.mongodb.org/mongo-driver/mongo/readpref" "go.mongodb.org/mongo-driver/mongo/writeconcern" - "go.mongodb.org/mongo-driver/tag" "go.mongodb.org/mongo-driver/x/mongo/driver/mongocrypt" "go.mongodb.org/mongo-driver/x/mongo/driver/session" "go.mongodb.org/mongo-driver/x/mongo/driver/topology" @@ -88,22 +88,22 @@ func TestClient(t *testing.T) { t.Run("read preference", func(t *testing.T) { t.Run("absent", func(t *testing.T) { client := setupClient() - gotMode := client.readPreference.Mode() + gotMode := client.readPreference.Mode wantMode := readpref.PrimaryMode assert.Equal(t, gotMode, wantMode, "expected mode %v, got %v", wantMode, gotMode) - _, flag := client.readPreference.MaxStaleness() - assert.False(t, flag, "expected max staleness to not be set but was") + gotMaxStaleness := client.readPreference.MaxStaleness + assert.Nil(t, gotMaxStaleness, "expected max staleness to not be set but was") }) t.Run("specified", func(t *testing.T) { - tags := []tag.Set{ + tags := []readpref.TagSet{ { - tag.Tag{ + readpref.Tag{ Name: "one", Value: "1", }, }, { - tag.Tag{ + readpref.Tag{ Name: "two", Value: "2", }, @@ -113,14 +113,14 @@ func TestClient(t *testing.T) { cs += "?readpreference=secondary&readPreferenceTags=one:1&readPreferenceTags=two:2&maxStaleness=5" client := setupClient(options.Client().ApplyURI(cs)) - gotMode := client.readPreference.Mode() + gotMode := client.readPreference.Mode assert.Equal(t, gotMode, readpref.SecondaryMode, "expected mode %v, got %v", readpref.SecondaryMode, gotMode) - gotTags := client.readPreference.TagSets() + gotTags := client.readPreference.TagSets assert.Equal(t, gotTags, tags, "expected tags %v, got %v", tags, gotTags) - gotStaleness, flag := client.readPreference.MaxStaleness() - assert.True(t, flag, "expected max staleness to be set but was not") + gotStaleness := client.readPreference.MaxStaleness + require.NotNil(t, gotStaleness, "expected max staleness to be set but was not") wantStaleness := time.Duration(5) * time.Second - assert.Equal(t, gotStaleness, wantStaleness, "expected staleness %v, got %v", wantStaleness, gotStaleness) + assert.Equal(t, *gotStaleness, wantStaleness, "expected staleness %v, got %v", wantStaleness, *gotStaleness) }) }) t.Run("localThreshold", func(t *testing.T) { diff --git a/mongo/database.go b/mongo/database.go index 36296a11b7..a782454425 100644 --- a/mongo/database.go +++ b/mongo/database.go @@ -183,7 +183,7 @@ func (db *Database) processRunCommand(ctx context.Context, cmd interface{}, ro.ReadPreference = opt.ReadPreference } } - if sess != nil && sess.TransactionRunning() && ro.ReadPreference != nil && ro.ReadPreference.Mode() != readpref.PrimaryMode { + if sess != nil && sess.TransactionRunning() && ro.ReadPreference != nil && ro.ReadPreference.Mode != readpref.PrimaryMode { return nil, sess, errors.New("read preference in a transaction must be primary") } diff --git a/mongo/options/clientoptions.go b/mongo/options/clientoptions.go index aee3df998b..e261008f3f 100644 --- a/mongo/options/clientoptions.go +++ b/mongo/options/clientoptions.go @@ -28,7 +28,6 @@ import ( "go.mongodb.org/mongo-driver/mongo/readconcern" "go.mongodb.org/mongo-driver/mongo/readpref" "go.mongodb.org/mongo-driver/mongo/writeconcern" - "go.mongodb.org/mongo-driver/tag" "go.mongodb.org/mongo-driver/x/mongo/driver" "go.mongodb.org/mongo-driver/x/mongo/driver/connstring" "go.mongodb.org/mongo-driver/x/mongo/driver/wiremessage" @@ -432,26 +431,23 @@ func (c *ClientOptions) ApplyURI(uri string) *ClientOptions { } if cs.ReadPreference != "" || len(cs.ReadPreferenceTagSets) > 0 || cs.MaxStalenessSet { - opts := make([]readpref.Option, 0, 1) - - tagSets := tag.NewTagSetsFromMaps(cs.ReadPreferenceTagSets) - if len(tagSets) > 0 { - opts = append(opts, readpref.WithTagSets(tagSets...)) - } - - if cs.MaxStaleness != 0 { - opts = append(opts, readpref.WithMaxStaleness(cs.MaxStaleness)) - } - mode, err := readpref.ModeFromString(cs.ReadPreference) if err != nil { c.err = err return c } - c.ReadPreference, c.err = readpref.New(mode, opts...) - if c.err != nil { - return c + c.ReadPreference = &readpref.ReadPref{ + Mode: mode, + } + + tagSets := readpref.NewTagSetsFromMaps(cs.ReadPreferenceTagSets) + if len(tagSets) > 0 { + c.ReadPreference.TagSets = tagSets + } + + if cs.MaxStaleness != 0 { + c.ReadPreference.MaxStaleness = &cs.MaxStaleness } } diff --git a/mongo/options/clientoptions_test.go b/mongo/options/clientoptions_test.go index 70131ded57..c17fb28660 100644 --- a/mongo/options/clientoptions_test.go +++ b/mongo/options/clientoptions_test.go @@ -211,15 +211,6 @@ func TestClientOptions(t *testing.T) { HTTPClient: httputil.DefaultHTTPClient, }, }, - { - "ReadPreference Primary With Options", - "mongodb://localhost/?readPreference=Primary&maxStaleness=200", - &ClientOptions{ - err: errors.New("can not specify tags, max staleness, or hedge with mode primary"), - Hosts: []string{"localhost"}, - HTTPClient: httputil.DefaultHTTPClient, - }, - }, { "TLS addCertFromFile error", "mongodb://localhost/?ssl=true&sslCertificateAuthorityFile=testdata/doesntexist", @@ -367,12 +358,26 @@ func TestClientOptions(t *testing.T) { { "ReadPreferenceTagSets", "mongodb://localhost/?readPreference=secondaryPreferred&readPreferenceTags=foo:bar", - baseClient().SetReadPreference(readpref.SecondaryPreferred(readpref.WithTags("foo", "bar"))), + baseClient().SetReadPreference(func() *readpref.ReadPref { + rp := readpref.SecondaryPreferred() //readpref.NewTagSet("foo", "bar") + + tagSet, _ := readpref.NewTagSet("foo", "bar") + rp.TagSets = append(rp.TagSets, tagSet) + + return rp + }()), }, { "MaxStaleness", "mongodb://localhost/?readPreference=secondaryPreferred&maxStaleness=250", - baseClient().SetReadPreference(readpref.SecondaryPreferred(readpref.WithMaxStaleness(250 * time.Second))), + baseClient().SetReadPreference(func() *readpref.ReadPref { + rp := readpref.SecondaryPreferred() + + maxStaleness := 250 * time.Second + rp.MaxStaleness = &maxStaleness + + return rp + }()), }, { "RetryWrites", diff --git a/mongo/readpref/mode.go b/mongo/readpref/mode.go index ce036504cb..deacf9f337 100644 --- a/mongo/readpref/mode.go +++ b/mongo/readpref/mode.go @@ -72,17 +72,3 @@ func (mode Mode) String() string { return "unknown" } } - -// IsValid checks whether the mode is valid. -func (mode Mode) IsValid() bool { - switch mode { - case PrimaryMode, - PrimaryPreferredMode, - SecondaryMode, - SecondaryPreferredMode, - NearestMode: - return true - default: - return false - } -} diff --git a/mongo/readpref/options.go b/mongo/readpref/options.go deleted file mode 100644 index c59b0705f1..0000000000 --- a/mongo/readpref/options.go +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright (C) MongoDB, Inc. 2017-present. -// -// Licensed under the Apache License, Version 2.0 (the "License"); you may -// not use this file except in compliance with the License. You may obtain -// a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - -package readpref - -import ( - "errors" - "time" - - "go.mongodb.org/mongo-driver/tag" -) - -// ErrInvalidTagSet indicates that an invalid set of tags was specified. -var ErrInvalidTagSet = errors.New("an even number of tags must be specified") - -// Option configures a read preference -type Option func(*ReadPref) error - -// WithMaxStaleness sets the maximum staleness a -// server is allowed. -func WithMaxStaleness(ms time.Duration) Option { - return func(rp *ReadPref) error { - rp.maxStaleness = ms - rp.maxStalenessSet = true - return nil - } -} - -// WithTags specifies a single tag set used to match replica set members. If no members match the -// tag set, read operations will return an error. To avoid errors if no members match the tag set, use -// [WithTagSets] and include an empty tag set as the last tag set in the list. -// -// The last call to [WithTags] or [WithTagSets] overrides all previous calls to either method. -// -// For more information about read preference tags, see -// https://www.mongodb.com/docs/manual/core/read-preference-tags/ -func WithTags(tags ...string) Option { - return func(rp *ReadPref) error { - length := len(tags) - if length < 2 || length%2 != 0 { - return ErrInvalidTagSet - } - - tagset := make(tag.Set, 0, length/2) - - for i := 1; i < length; i += 2 { - tagset = append(tagset, tag.Tag{Name: tags[i-1], Value: tags[i]}) - } - - return WithTagSets(tagset)(rp) - } -} - -// WithTagSets specifies a list of tag sets used to match replica set members. If the list contains -// multiple tag sets, members are matched against each tag set in succession until a match is found. -// Once a match is found, the remaining tag sets are ignored. If no members match any of the tag -// sets, the read operation returns with an error. To avoid an error if no members match any of the -// tag sets, include an empty tag set as the last tag set in the list. -// -// The last call to [WithTags] or [WithTagSets] overrides all previous calls to either method. -// -// For more information about read preference tags, see -// https://www.mongodb.com/docs/manual/core/read-preference-tags/ -func WithTagSets(tagSets ...tag.Set) Option { - return func(rp *ReadPref) error { - rp.tagSets = tagSets - return nil - } -} - -// WithHedgeEnabled specifies whether or not hedged reads should be enabled in the server. This feature requires MongoDB -// server version 4.4 or higher. For more information about hedged reads, see -// https://www.mongodb.com/docs/manual/core/sharded-cluster-query-router/#mongos-hedged-reads. If not specified, the default -// is to not send a value to the server, which will result in the server defaults being used. -func WithHedgeEnabled(hedgeEnabled bool) Option { - return func(rp *ReadPref) error { - rp.hedgeEnabled = &hedgeEnabled - return nil - } -} diff --git a/mongo/readpref/options_example_test.go b/mongo/readpref/options_example_test.go deleted file mode 100644 index bdff555d34..0000000000 --- a/mongo/readpref/options_example_test.go +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright (C) MongoDB, Inc. 2023-present. -// -// Licensed under the Apache License, Version 2.0 (the "License"); you may -// not use this file except in compliance with the License. You may obtain -// a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - -package readpref_test - -import ( - "go.mongodb.org/mongo-driver/mongo" - "go.mongodb.org/mongo-driver/mongo/options" - "go.mongodb.org/mongo-driver/mongo/readpref" - "go.mongodb.org/mongo-driver/tag" -) - -// Configure a Client with a read preference that selects the nearest replica -// set member that includes tags "region: South" and "datacenter: A". -func ExampleWithTags() { - rp := readpref.Nearest( - readpref.WithTags( - "region", "South", - "datacenter", "A")) - - opts := options.Client(). - ApplyURI("mongodb://localhost:27017"). - SetReadPreference(rp) - - _, err := mongo.Connect(opts) - if err != nil { - panic(err) - } -} - -// Configure a Client with a read preference that selects the nearest replica -// set member matching a set of tags. Try to match members in 3 stages: -// -// 1. Match replica set members that include tags "region: South" and -// "datacenter: A". -// 2. Match replica set members that includes tag "region: South". -// 3. Match any replica set member. -// -// Stage 3 is used to avoid errors when no members match the previous 2 stages. -func ExampleWithTagSets() { - tagSetList := tag.NewTagSetsFromMaps([]map[string]string{ - {"region": "South", "datacenter": "A"}, - {"region": "South"}, - {}, - }) - - rp := readpref.Nearest(readpref.WithTagSets(tagSetList...)) - - opts := options.Client(). - ApplyURI("mongodb://localhost"). - SetReadPreference(rp) - - _, err := mongo.Connect(opts) - if err != nil { - panic(err) - } -} diff --git a/mongo/readpref/readpref.go b/mongo/readpref/readpref.go index 40c6ca76da..58409a4d58 100644 --- a/mongo/readpref/readpref.go +++ b/mongo/readpref/readpref.go @@ -9,121 +9,66 @@ package readpref import ( "bytes" - "errors" "fmt" "time" - - "go.mongodb.org/mongo-driver/tag" -) - -var ( - errInvalidReadPreference = errors.New("can not specify tags, max staleness, or hedge with mode primary") ) // Primary constructs a read preference with a PrimaryMode. func Primary() *ReadPref { - return &ReadPref{mode: PrimaryMode} + return &ReadPref{Mode: PrimaryMode} } // PrimaryPreferred constructs a read preference with a PrimaryPreferredMode. -func PrimaryPreferred(opts ...Option) *ReadPref { - // New only returns an error with a mode of Primary - rp, _ := New(PrimaryPreferredMode, opts...) - return rp +func PrimaryPreferred() *ReadPref { + return &ReadPref{Mode: PrimaryPreferredMode} } // SecondaryPreferred constructs a read preference with a SecondaryPreferredMode. -func SecondaryPreferred(opts ...Option) *ReadPref { - // New only returns an error with a mode of Primary - rp, _ := New(SecondaryPreferredMode, opts...) - return rp +func SecondaryPreferred() *ReadPref { + return &ReadPref{Mode: SecondaryPreferredMode} } // Secondary constructs a read preference with a SecondaryMode. -func Secondary(opts ...Option) *ReadPref { - // New only returns an error with a mode of Primary - rp, _ := New(SecondaryMode, opts...) - return rp +func Secondary() *ReadPref { + return &ReadPref{Mode: SecondaryMode} } // Nearest constructs a read preference with a NearestMode. -func Nearest(opts ...Option) *ReadPref { - // New only returns an error with a mode of Primary - rp, _ := New(NearestMode, opts...) - return rp -} - -// New creates a new ReadPref. -func New(mode Mode, opts ...Option) (*ReadPref, error) { - rp := &ReadPref{ - mode: mode, - } - - if mode == PrimaryMode && len(opts) != 0 { - return nil, errInvalidReadPreference - } - - for _, opt := range opts { - if opt == nil { - continue - } - err := opt(rp) - if err != nil { - return nil, err - } - } - - return rp, nil +func Nearest() *ReadPref { + return &ReadPref{Mode: NearestMode} } // ReadPref determines which servers are considered suitable for read operations. type ReadPref struct { - maxStaleness time.Duration - maxStalenessSet bool - mode Mode - tagSets []tag.Set - hedgeEnabled *bool -} + // Maximum amount of time to allow a server to be considered eligible for + // selection. The second return value indicates if this value has been set. + MaxStaleness *time.Duration -// MaxStaleness is the maximum amount of time to allow -// a server to be considered eligible for selection. The -// second return value indicates if this value has been set. -func (r *ReadPref) MaxStaleness() (time.Duration, bool) { - return r.maxStaleness, r.maxStalenessSet -} - -// Mode indicates the mode of the read preference. -func (r *ReadPref) Mode() Mode { - return r.mode -} + // Mode indicates the mode of the read preference. + Mode Mode -// TagSets are multiple tag sets indicating -// which servers should be considered. -func (r *ReadPref) TagSets() []tag.Set { - return r.tagSets -} + // Tag sets indicating which servers should be considered. + TagSets []TagSet -// HedgeEnabled returns whether or not hedged reads are enabled for this read preference. If this option was not -// specified during read preference construction, nil is returned. -func (r *ReadPref) HedgeEnabled() *bool { - return r.hedgeEnabled + // Specify whether or not hedged reads are enabled for this read preference. + HedgeEnabled *bool } // String returns a human-readable description of the read preference. func (r *ReadPref) String() string { var b bytes.Buffer - b.WriteString(r.mode.String()) + b.WriteString(r.Mode.String()) delim := "(" - if r.maxStalenessSet { - fmt.Fprintf(&b, "%smaxStaleness=%v", delim, r.maxStaleness) + if r.MaxStaleness != nil { + fmt.Fprintf(&b, "%smaxStaleness=%v", delim, r.MaxStaleness) delim = " " } - for _, tagSet := range r.tagSets { + for _, tagSet := range r.TagSets { fmt.Fprintf(&b, "%stagSet=%s", delim, tagSet.String()) delim = " " } - if r.hedgeEnabled != nil { - fmt.Fprintf(&b, "%shedgeEnabled=%v", delim, *r.hedgeEnabled) + if r.HedgeEnabled != nil { + fmt.Fprintf(&b, "%shedgeEnabled=%v", delim, *r.HedgeEnabled) delim = " " } if delim != "(" { diff --git a/mongo/readpref/readpref_test.go b/mongo/readpref/readpref_test.go index d1ccb4554d..73c1d29ec6 100644 --- a/mongo/readpref/readpref_test.go +++ b/mongo/readpref/readpref_test.go @@ -11,133 +11,31 @@ import ( "time" "go.mongodb.org/mongo-driver/internal/assert" - "go.mongodb.org/mongo-driver/internal/require" - "go.mongodb.org/mongo-driver/tag" ) -func TestPrimary(t *testing.T) { - subject := Primary() - - require.Equal(t, PrimaryMode, subject.Mode()) - _, set := subject.MaxStaleness() - require.False(t, set) - require.Len(t, subject.TagSets(), 0) -} - -func TestPrimaryPreferred(t *testing.T) { - subject := PrimaryPreferred() - - require.Equal(t, PrimaryPreferredMode, subject.Mode()) - _, set := subject.MaxStaleness() - require.False(t, set) - require.Len(t, subject.TagSets(), 0) -} - -func TestPrimaryPreferred_with_options(t *testing.T) { - subject := PrimaryPreferred( - WithMaxStaleness(time.Duration(10)), - WithTags("a", "1", "b", "2"), - ) - - require.Equal(t, PrimaryPreferredMode, subject.Mode()) - ms, set := subject.MaxStaleness() - require.True(t, set) - require.Equal(t, time.Duration(10), ms) - require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) -} - -func TestSecondaryPreferred(t *testing.T) { - subject := SecondaryPreferred() - - require.Equal(t, SecondaryPreferredMode, subject.Mode()) - _, set := subject.MaxStaleness() - require.False(t, set) - require.Len(t, subject.TagSets(), 0) -} - -func TestSecondaryPreferred_with_options(t *testing.T) { - subject := SecondaryPreferred( - WithMaxStaleness(time.Duration(10)), - WithTags("a", "1", "b", "2"), - ) - - require.Equal(t, SecondaryPreferredMode, subject.Mode()) - ms, set := subject.MaxStaleness() - require.True(t, set) - require.Equal(t, time.Duration(10), ms) - require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) -} - -func TestSecondary(t *testing.T) { - subject := Secondary() - - require.Equal(t, SecondaryMode, subject.Mode()) - _, set := subject.MaxStaleness() - require.False(t, set) - require.Len(t, subject.TagSets(), 0) -} - -func TestSecondary_with_options(t *testing.T) { - subject := Secondary( - WithMaxStaleness(time.Duration(10)), - WithTags("a", "1", "b", "2"), - ) - - require.Equal(t, SecondaryMode, subject.Mode()) - ms, set := subject.MaxStaleness() - require.True(t, set) - require.Equal(t, time.Duration(10), ms) - require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) -} - -func TestNearest(t *testing.T) { - subject := Nearest() +func TestReadPref_String(t *testing.T) { + t.Run("ReadPref.String() with all options", func(t *testing.T) { + readPref := Nearest() - require.Equal(t, NearestMode, subject.Mode()) - _, set := subject.MaxStaleness() - require.False(t, set) - require.Len(t, subject.TagSets(), 0) -} + maxStaleness := 120 * time.Second + readPref.MaxStaleness = &maxStaleness -func TestNearest_with_options(t *testing.T) { - subject := Nearest( - WithMaxStaleness(time.Duration(10)), - WithTags("a", "1", "b", "2"), - ) + readPref.TagSets = []TagSet{{{"a", "1"}, {"b", "2"}}, {{"q", "5"}, {"r", "6"}}} - require.Equal(t, NearestMode, subject.Mode()) - ms, set := subject.MaxStaleness() - require.True(t, set) - require.Equal(t, time.Duration(10), ms) - require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) -} + hedgeEnabled := true + readPref.HedgeEnabled = &hedgeEnabled -func TestHedge(t *testing.T) { - t.Run("hedge specified with primary mode errors", func(t *testing.T) { - _, err := New(PrimaryMode, WithHedgeEnabled(true)) - assert.Equal(t, errInvalidReadPreference, err, "expected error %v, got %v", errInvalidReadPreference, err) - }) - t.Run("valid hedge document and mode succeeds", func(t *testing.T) { - rp, err := New(SecondaryMode, WithHedgeEnabled(true)) - assert.Nil(t, err, "expected no error, got %v", err) - enabled := rp.HedgeEnabled() - assert.NotNil(t, enabled, "expected HedgeEnabled to return a non-nil value, got nil") - assert.True(t, *enabled, "expected HedgeEnabled to return true, got false") - }) -} - -func TestReadPref_String(t *testing.T) { - t.Run("ReadPref.String() with all options", func(t *testing.T) { - readPref := Nearest( - WithMaxStaleness(120*time.Second), - WithTagSets(tag.Set{{"a", "1"}, {"b", "2"}}, tag.Set{{"q", "5"}, {"r", "6"}}), - WithHedgeEnabled(true), - ) expected := "nearest(maxStaleness=2m0s tagSet=a=1,b=2 tagSet=q=5,r=6 hedgeEnabled=true)" assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) }) t.Run("ReadPref.String() with one option", func(t *testing.T) { - readPref := Secondary(WithTags("a", "1", "b", "2")) + readPref := Secondary() + + tagSet, err := NewTagSet("a", "1", "b", "2") + assert.NoError(t, err) + + readPref.TagSets = append(readPref.TagSets, tagSet) + expected := "secondary(tagSet=a=1,b=2)" assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) }) diff --git a/tag/tag.go b/mongo/readpref/tag.go similarity index 66% rename from tag/tag.go rename to mongo/readpref/tag.go index 39c11e0460..37e188511c 100644 --- a/tag/tag.go +++ b/mongo/readpref/tag.go @@ -4,11 +4,7 @@ // not use this file except in compliance with the License. You may obtain // a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 -// Package tag provides types for filtering replica set members using tags in a read preference. -// -// For more information about read preference tags, see -// https://www.mongodb.com/docs/manual/core/read-preference-tags/ -package tag +package readpref import ( "bytes" @@ -26,12 +22,36 @@ func (tag Tag) String() string { return fmt.Sprintf("%s=%s", tag.Name, tag.Value) } +// TagSet is an ordered list of Tags. +type TagSet []Tag + +// NewTagSet is a convenience function to specify a single tag set used to match +// replica set members. If no members match the tag set, read operations will +// return an error. +// +// For more information about read preference tags, see +// https://www.mongodb.com/docs/manual/core/read-preference-tags/ +func NewTagSet(tags ...string) (TagSet, error) { + length := len(tags) + if length < 2 || length%2 != 0 { + return nil, fmt.Errorf("an even number of tags must be specified") + } + + tagset := make(TagSet, 0, length/2) + + for i := 1; i < length; i += 2 { + tagset = append(tagset, Tag{Name: tags[i-1], Value: tags[i]}) + } + + return tagset, nil +} + // NewTagSetFromMap creates a tag set from a map. // // For more information about read preference tags, see // https://www.mongodb.com/docs/manual/core/read-preference-tags/ -func NewTagSetFromMap(m map[string]string) Set { - var set Set +func NewTagSetFromMap(m map[string]string) TagSet { + set := make(TagSet, 0, len(m)) for k, v := range m { set = append(set, Tag{Name: k, Value: v}) } @@ -43,19 +63,16 @@ func NewTagSetFromMap(m map[string]string) Set { // // For more information about read preference tags, see // https://www.mongodb.com/docs/manual/core/read-preference-tags/ -func NewTagSetsFromMaps(maps []map[string]string) []Set { - sets := make([]Set, 0, len(maps)) +func NewTagSetsFromMaps(maps []map[string]string) []TagSet { + sets := make([]TagSet, 0, len(maps)) for _, m := range maps { sets = append(sets, NewTagSetFromMap(m)) } return sets } -// Set is an ordered list of Tags. -type Set []Tag - // Contains indicates whether the name/value pair exists in the tagset. -func (ts Set) Contains(name, value string) bool { +func (ts TagSet) Contains(name, value string) bool { for _, t := range ts { if t.Name == name && t.Value == value { return true @@ -66,7 +83,7 @@ func (ts Set) Contains(name, value string) bool { } // ContainsAll indicates whether all the name/value pairs exist in the tagset. -func (ts Set) ContainsAll(other []Tag) bool { +func (ts TagSet) ContainsAll(other []Tag) bool { for _, ot := range other { if !ts.Contains(ot.Name, ot.Value) { return false @@ -77,7 +94,7 @@ func (ts Set) ContainsAll(other []Tag) bool { } // String returns a human-readable human-readable description of the tagset. -func (ts Set) String() string { +func (ts TagSet) String() string { var b bytes.Buffer for i, tag := range ts { if i > 0 { diff --git a/tag/tag_test.go b/mongo/readpref/tag_test.go similarity index 81% rename from tag/tag_test.go rename to mongo/readpref/tag_test.go index 45f29ad866..d8dc06e4d7 100644 --- a/tag/tag_test.go +++ b/mongo/readpref/tag_test.go @@ -4,7 +4,7 @@ // not use this file except in compliance with the License. You may obtain // a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 -package tag +package readpref import ( "testing" @@ -23,7 +23,7 @@ func TestTag_String(t *testing.T) { func TestTagSets_NewTagSet(t *testing.T) { t.Parallel() - ts := Set{Tag{Name: "a", Value: "1"}} + ts := TagSet{Tag{Name: "a", Value: "1"}} require.True(t, ts.Contains("a", "1")) require.False(t, ts.Contains("1", "a")) @@ -65,30 +65,30 @@ func TestTagSets_NewTagSetsFromMaps(t *testing.T) { func TestTagSets_ContainsAll(t *testing.T) { t.Parallel() - ts := Set{ + ts := TagSet{ Tag{Name: "a", Value: "1"}, Tag{Name: "b", Value: "2"}, } - test := Set{Tag{Name: "a", Value: "1"}} + test := TagSet{Tag{Name: "a", Value: "1"}} require.True(t, ts.ContainsAll(test)) - test = Set{Tag{Name: "a", Value: "1"}, Tag{Name: "b", Value: "2"}} + test = TagSet{Tag{Name: "a", Value: "1"}, Tag{Name: "b", Value: "2"}} require.True(t, ts.ContainsAll(test)) - test = Set{Tag{Name: "a", Value: "1"}, Tag{Name: "b", Value: "2"}} + test = TagSet{Tag{Name: "a", Value: "1"}, Tag{Name: "b", Value: "2"}} require.True(t, ts.ContainsAll(test)) - test = Set{Tag{Name: "a", Value: "2"}, Tag{Name: "b", Value: "1"}} + test = TagSet{Tag{Name: "a", Value: "2"}, Tag{Name: "b", Value: "1"}} require.False(t, ts.ContainsAll(test)) - test = Set{Tag{Name: "a", Value: "1"}, Tag{Name: "b", Value: "1"}} + test = TagSet{Tag{Name: "a", Value: "1"}, Tag{Name: "b", Value: "1"}} require.False(t, ts.ContainsAll(test)) - test = Set{Tag{Name: "a", Value: "2"}, Tag{Name: "b", Value: "2"}} + test = TagSet{Tag{Name: "a", Value: "2"}, Tag{Name: "b", Value: "2"}} require.False(t, ts.ContainsAll(test)) } func TestTagSets_String(t *testing.T) { t.Parallel() - ts := Set{ + ts := TagSet{ Tag{Name: "a", Value: "1"}, Tag{Name: "b", Value: "2"}, } diff --git a/x/mongo/driver/description/server.go b/x/mongo/driver/description/server.go index a5d9943114..f018dedc9d 100644 --- a/x/mongo/driver/description/server.go +++ b/x/mongo/driver/description/server.go @@ -12,7 +12,7 @@ import ( "go.mongodb.org/mongo-driver/bson" "go.mongodb.org/mongo-driver/mongo/address" - "go.mongodb.org/mongo-driver/tag" + "go.mongodb.org/mongo-driver/mongo/readpref" ) // ServerKind represents the type of a single server in a topology. @@ -105,7 +105,7 @@ type Server struct { SessionTimeoutMinutes *int64 SetName string SetVersion uint32 - Tags tag.Set + Tags readpref.TagSet TopologyVersion *TopologyVersion Kind ServerKind WireVersion *VersionRange diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index 61110e5467..902008770b 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -1697,7 +1697,7 @@ func (op Operation) getReadPrefBasedOnTransaction() (*readpref.ReadPref, error) rp := op.Client.CurrentRp // Reads in a transaction must have read preference primary // This must not be checked in startTransaction - if rp != nil && !op.Client.TransactionStarting() && rp.Mode() != readpref.PrimaryMode { + if rp != nil && !op.Client.TransactionStarting() && rp.Mode != readpref.PrimaryMode { return nil, ErrNonPrimaryReadPref } return rp, nil @@ -1743,7 +1743,7 @@ func (op Operation) createReadPref(desc description.SelectedServer, isOpQuery bo return nil, nil } - switch rp.Mode() { + switch rp.Mode { case readpref.PrimaryMode: if desc.Server.Kind == description.ServerKindMongos { return nil, nil @@ -1764,9 +1764,9 @@ func (op Operation) createReadPref(desc description.SelectedServer, isOpQuery bo case readpref.PrimaryPreferredMode: doc = bsoncore.AppendStringElement(doc, "mode", "primaryPreferred") case readpref.SecondaryPreferredMode: - _, ok := rp.MaxStaleness() - if desc.Server.Kind == description.ServerKindMongos && isOpQuery && !ok && len(rp.TagSets()) == 0 && - rp.HedgeEnabled() == nil { + ok := rp.MaxStaleness != nil + if desc.Server.Kind == description.ServerKindMongos && isOpQuery && !ok && len(rp.TagSets) == 0 && + rp.HedgeEnabled == nil { return nil, nil } @@ -1777,8 +1777,8 @@ func (op Operation) createReadPref(desc description.SelectedServer, isOpQuery bo doc = bsoncore.AppendStringElement(doc, "mode", "nearest") } - sets := make([]bsoncore.Document, 0, len(rp.TagSets())) - for _, ts := range rp.TagSets() { + sets := make([]bsoncore.Document, 0, len(rp.TagSets)) + for _, ts := range rp.TagSets { i, set := bsoncore.AppendDocumentStart(nil) for _, t := range ts { set = bsoncore.AppendStringElement(set, t.Name, t.Value) @@ -1795,11 +1795,11 @@ func (op Operation) createReadPref(desc description.SelectedServer, isOpQuery bo doc, _ = bsoncore.AppendArrayEnd(doc, aidx) } - if d, ok := rp.MaxStaleness(); ok { - doc = bsoncore.AppendInt32Element(doc, "maxStalenessSeconds", int32(d.Seconds())) + if maxStaleness := rp.MaxStaleness; maxStaleness != nil { + doc = bsoncore.AppendInt32Element(doc, "maxStalenessSeconds", int32((*maxStaleness).Seconds())) } - if hedgeEnabled := rp.HedgeEnabled(); hedgeEnabled != nil { + if hedgeEnabled := rp.HedgeEnabled; hedgeEnabled != nil { var hedgeIdx int32 hedgeIdx, doc = bsoncore.AppendDocumentElementStart(doc, "hedge") doc = bsoncore.AppendBooleanElement(doc, "enabled", *hedgeEnabled) @@ -1818,7 +1818,7 @@ func (op Operation) secondaryOK(desc description.SelectedServer) wiremessage.Que return wiremessage.SecondaryOK } - if rp := op.ReadPreference; rp != nil && rp.Mode() != readpref.PrimaryMode { + if rp := op.ReadPreference; rp != nil && rp.Mode != readpref.PrimaryMode { return wiremessage.SecondaryOK } diff --git a/x/mongo/driver/operation_test.go b/x/mongo/driver/operation_test.go index f209134b79..4de77cc45b 100644 --- a/x/mongo/driver/operation_test.go +++ b/x/mongo/driver/operation_test.go @@ -24,7 +24,6 @@ import ( "go.mongodb.org/mongo-driver/mongo/readconcern" "go.mongodb.org/mongo-driver/mongo/readpref" "go.mongodb.org/mongo-driver/mongo/writeconcern" - "go.mongodb.org/mongo-driver/tag" "go.mongodb.org/mongo-driver/x/bsonx/bsoncore" "go.mongodb.org/mongo-driver/x/mongo/driver/description" "go.mongodb.org/mongo-driver/x/mongo/driver/mnet" @@ -445,7 +444,16 @@ func TestOperation(t *testing.T) { {"nearest", readpref.Nearest(), description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpNearest}, { "secondaryPreferred/withTags", - readpref.SecondaryPreferred(readpref.WithTags("disk", "ssd", "use", "reporting")), + func() *readpref.ReadPref { + rp := readpref.SecondaryPreferred() + + tagSet, err := readpref.NewTagSet("disk", "ssd", "use", "reporting") + assert.NoError(t, err) + + rp.TagSets = []readpref.TagSet{tagSet} + + return rp + }(), description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpWithTags, }, // GODRIVER-2205: Ensure empty tag sets are written as an empty document in the read @@ -453,9 +461,15 @@ func TestOperation(t *testing.T) { // no other tag sets match any servers. { "secondaryPreferred/withTags/emptyTagSet", - readpref.SecondaryPreferred(readpref.WithTagSets( - tag.Set{{Name: "disk", Value: "ssd"}}, - tag.Set{})), + func() *readpref.ReadPref { + rp := readpref.SecondaryPreferred() + rp.TagSets = []readpref.TagSet{ + readpref.TagSet{{Name: "disk", Value: "ssd"}}, + readpref.TagSet{}, + } + + return rp + }(), description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, @@ -469,13 +483,27 @@ func TestOperation(t *testing.T) { }, { "secondaryPreferred/withMaxStaleness", - readpref.SecondaryPreferred(readpref.WithMaxStaleness(25 * time.Second)), + func() *readpref.ReadPref { + rp := readpref.SecondaryPreferred() + + maxStaleness := 25 * time.Second + rp.MaxStaleness = &maxStaleness + + return rp + }(), description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpWithMaxStaleness, }, { // A read preference document is generated for SecondaryPreferred if the hedge document is non-nil. "secondaryPreferred with hedge to mongos using OP_QUERY", - readpref.SecondaryPreferred(readpref.WithHedgeEnabled(true)), + func() *readpref.ReadPref { + rp := readpref.SecondaryPreferred() + + he := true + rp.HedgeEnabled = &he + + return rp + }(), description.ServerKindMongos, description.TopologyKindSharded, true, @@ -483,11 +511,22 @@ func TestOperation(t *testing.T) { }, { "secondaryPreferred with all options", - readpref.SecondaryPreferred( - readpref.WithTags("disk", "ssd", "use", "reporting"), - readpref.WithMaxStaleness(25*time.Second), - readpref.WithHedgeEnabled(false), - ), + func() *readpref.ReadPref { + rp := readpref.SecondaryPreferred() + + tagSet, err := readpref.NewTagSet("disk", "ssd", "use", "reporting") + assert.NoError(t, err) + + rp.TagSets = []readpref.TagSet{tagSet} + + maxStaleness := 25 * time.Second + rp.MaxStaleness = &maxStaleness + + he := false + rp.HedgeEnabled = &he + + return rp + }(), description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, From 101056af7ad8db9db629cc7b9cd10b516f777665 Mon Sep 17 00:00:00 2001 From: Preston Vasquez Date: Tue, 23 Jul 2024 13:22:33 -0600 Subject: [PATCH 2/4] GODRIVER-2689 Make updates --- internal/docexamples/examples.go | 4 +- internal/integration/database_test.go | 2 +- .../initial_dns_seedlist_discovery_test.go | 2 +- internal/integration/json_helpers_test.go | 8 +- internal/integration/mtest/mongotest.go | 2 +- .../integration/unified/common_options.go | 10 +- internal/serverselector/server_selector.go | 12 +- .../serverselector/server_selector_test.go | 162 ++++++++++++------ mongo/client.go | 4 +- mongo/client_test.go | 2 +- mongo/collection_test.go | 2 +- mongo/crud_examples_test.go | 5 +- mongo/database.go | 4 +- mongo/database_test.go | 2 +- mongo/options/clientoptions.go | 26 +-- mongo/options/clientoptions_test.go | 18 +- mongo/readpref/readpref.go | 90 ++++++---- mongo/readpref/readpref_test.go | 18 +- x/mongo/driver/operation.go | 14 +- x/mongo/driver/operation_test.go | 49 ++++-- x/mongo/driver/topology/topology_test.go | 2 +- 21 files changed, 275 insertions(+), 163 deletions(-) diff --git a/internal/docexamples/examples.go b/internal/docexamples/examples.go index 71064947a9..64955a8849 100644 --- a/internal/docexamples/examples.go +++ b/internal/docexamples/examples.go @@ -2591,7 +2591,9 @@ func CausalConsistencyExamples(client *mongo.Client) error { // Start Causal Consistency Example 2 // Make a new session that is causally consistent with session1 so session2 reads what session1 writes - opts = options.Session().SetDefaultReadPreference(readpref.Secondary()). + secondaryRP := &readpref.ReadPref{Mode: readpref.SecondaryMode} + + opts = options.Session().SetDefaultReadPreference(secondaryRP). SetDefaultReadConcern(rc).SetDefaultWriteConcern(wc) session2, err := client.StartSession(opts) if err != nil { diff --git a/internal/integration/database_test.go b/internal/integration/database_test.go index 56505a0525..c8a58255a9 100644 --- a/internal/integration/database_test.go +++ b/internal/integration/database_test.go @@ -74,7 +74,7 @@ func TestDatabase(t *testing.T) { // layer, which should add a top-level $readPreference field to the command. runCmdOpts := options.RunCmd(). - SetReadPreference(readpref.SecondaryPreferred()) + SetReadPreference(&readpref.ReadPref{Mode: readpref.SecondaryPreferredMode}) err := mt.DB.RunCommand(context.Background(), bson.D{{handshake.LegacyHello, 1}}, runCmdOpts).Err() assert.Nil(mt, err, "RunCommand error: %v", err) diff --git a/internal/integration/initial_dns_seedlist_discovery_test.go b/internal/integration/initial_dns_seedlist_discovery_test.go index 4a0e8ba738..5f67a1ba02 100644 --- a/internal/integration/initial_dns_seedlist_discovery_test.go +++ b/internal/integration/initial_dns_seedlist_discovery_test.go @@ -87,7 +87,7 @@ func runSeedlistDiscoveryPingTest(mt *mtest.T, clientOpts *options.ClientOptions defer cancel() // Ping the server. - err = client.Ping(pingCtx, readpref.Nearest()) + err = client.Ping(pingCtx, &readpref.ReadPref{Mode: readpref.NearestMode}) assert.Nil(mt, err, "Ping error: %v", err) } diff --git a/internal/integration/json_helpers_test.go b/internal/integration/json_helpers_test.go index 194c316413..fb510632c2 100644 --- a/internal/integration/json_helpers_test.go +++ b/internal/integration/json_helpers_test.go @@ -429,13 +429,13 @@ func readPrefFromString(s string) *readpref.ReadPref { case "primary": return readpref.Primary() case "primarypreferred": - return readpref.PrimaryPreferred() + return &readpref.ReadPref{Mode: readpref.PrimaryPreferredMode} case "secondary": - return readpref.Secondary() + return &readpref.ReadPref{Mode: readpref.SecondaryMode} case "secondarypreferred": - return readpref.SecondaryPreferred() + return &readpref.ReadPref{Mode: readpref.SecondaryPreferredMode} case "nearest": - return readpref.Nearest() + return &readpref.ReadPref{Mode: readpref.NearestMode} } return readpref.Primary() } diff --git a/internal/integration/mtest/mongotest.go b/internal/integration/mtest/mongotest.go index affa4233df..6e8d4ffcdf 100644 --- a/internal/integration/mtest/mongotest.go +++ b/internal/integration/mtest/mongotest.go @@ -34,7 +34,7 @@ var ( // PrimaryRp is the primary read preference. PrimaryRp = readpref.Primary() // SecondaryRp is the secondary read preference. - SecondaryRp = readpref.Secondary() + SecondaryRp = &readpref.ReadPref{Mode: readpref.SecondaryMode} // LocalRc is the local read concern LocalRc = readconcern.Local() // MajorityRc is the majority read concern diff --git a/internal/integration/unified/common_options.go b/internal/integration/unified/common_options.go index 1282f86c89..3a9e610345 100644 --- a/internal/integration/unified/common_options.go +++ b/internal/integration/unified/common_options.go @@ -69,7 +69,7 @@ func (rp *ReadPreference) ToReadPrefOption() (*readpref.ReadPref, error) { return nil, fmt.Errorf("invalid read preference mode %q", rp.Mode) } - newReadPref := &readpref.ReadPref{Mode: mode} + rpOpts := &readpref.Options{} if rp.TagSets != nil { // Each item in the TagSets slice is a document that represents one set. @@ -82,11 +82,11 @@ func (rp *ReadPreference) ToReadPrefOption() (*readpref.ReadPref, error) { sets = append(sets, parsed) } - newReadPref.TagSets = sets + rpOpts.TagSets = sets } if rp.MaxStalenessSeconds != nil { maxStaleness := time.Duration(*rp.MaxStalenessSeconds) * time.Second - newReadPref.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness } if rp.Hedge != nil { @@ -95,9 +95,9 @@ func (rp *ReadPreference) ToReadPrefOption() (*readpref.ReadPref, error) { } if enabled, ok := rp.Hedge["enabled"]; ok { hedgeEnabled := enabled.(bool) - newReadPref.HedgeEnabled = &hedgeEnabled + rpOpts.HedgeEnabled = &hedgeEnabled } } - return newReadPref, nil + return readpref.New(mode, rpOpts) } diff --git a/internal/serverselector/server_selector.go b/internal/serverselector/server_selector.go index 57fe1828f8..4964942f32 100644 --- a/internal/serverselector/server_selector.go +++ b/internal/serverselector/server_selector.go @@ -189,7 +189,7 @@ func (ssf Func) SelectServer( } func verifyMaxStaleness(rp *readpref.ReadPref, topo description.Topology) error { - maxStaleness := rp.MaxStaleness + maxStaleness := rp.MaxStaleness() if maxStaleness == nil { return nil } @@ -241,7 +241,7 @@ func selectSecondaries(rp *readpref.ReadPref, candidates []description.Server) [ if len(secondaries) == 0 { return secondaries } - if maxStaleness := rp.MaxStaleness; maxStaleness != nil { + if maxStaleness := rp.MaxStaleness(); maxStaleness != nil { primaries := selectByKind(candidates, description.ServerKindRSPrimary) if len(primaries) == 0 { baseTime := secondaries[0].LastWriteTime @@ -334,24 +334,24 @@ func selectForReplicaSet( if len(selected) == 0 { selected = selectSecondaries(rp, candidates) - return selectByTagSet(selected, rp.TagSets), nil + return selectByTagSet(selected, rp.TagSets()), nil } return selected, nil case readpref.SecondaryPreferredMode: selected := selectSecondaries(rp, candidates) - selected = selectByTagSet(selected, rp.TagSets) + selected = selectByTagSet(selected, rp.TagSets()) if len(selected) > 0 { return selected, nil } return selectByKind(candidates, description.ServerKindRSPrimary), nil case readpref.SecondaryMode: selected := selectSecondaries(rp, candidates) - return selectByTagSet(selected, rp.TagSets), nil + return selectByTagSet(selected, rp.TagSets()), nil case readpref.NearestMode: selected := selectByKind(candidates, description.ServerKindRSPrimary) selected = append(selected, selectSecondaries(rp, candidates)...) - return selectByTagSet(selected, rp.TagSets), nil + return selectByTagSet(selected, rp.TagSets()), nil } return nil, fmt.Errorf("unsupported mode: %d", rp.Mode) diff --git a/internal/serverselector/server_selector_test.go b/internal/serverselector/server_selector_test.go index 8c119cf49f..c7f84ebf26 100644 --- a/internal/serverselector/server_selector_test.go +++ b/internal/serverselector/server_selector_test.go @@ -242,18 +242,21 @@ func selectServers(t *testing.T, test *testCase) error { return err } - rp := &readpref.ReadPref{Mode: readprefMode} + rpOpts := &readpref.Options{} tagSets := readpref.NewTagSetsFromMaps(test.ReadPreference.TagSets) if anyTagsInSets(tagSets) { - rp.TagSets = tagSets + rpOpts.TagSets = tagSets } if test.ReadPreference.MaxStaleness != nil { s := time.Duration(*test.ReadPreference.MaxStaleness) * time.Second - rp.MaxStaleness = &s + rpOpts.MaxStaleness = &s } + rp, err := readpref.New(readprefMode, rpOpts) + assert.NoError(t, err) + var selector description.ServerSelector selector = &ReadPref{ReadPref: rp} @@ -749,7 +752,7 @@ func TestSelector_Primary_with_no_primary(t *testing.T) { func TestSelector_PrimaryPreferred(t *testing.T) { t.Parallel() - subject := readpref.PrimaryPreferred() + subject := &readpref.ReadPref{Mode: readpref.PrimaryPreferredMode} result, err := (&ReadPref{ReadPref: subject}). SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -762,12 +765,15 @@ func TestSelector_PrimaryPreferred(t *testing.T) { func TestSelector_PrimaryPreferred_ignores_tags(t *testing.T) { t.Parallel() - subject := readpref.PrimaryPreferred() + rpOpts := &readpref.Options{} tagSet, err := readpref.NewTagSet("a", "2") assert.NoError(t, err) - subject.TagSets = []readpref.TagSet{tagSet} + rpOpts.TagSets = []readpref.TagSet{tagSet} + + subject, err := readpref.New(readpref.PrimaryPreferredMode, rpOpts) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}). SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -780,7 +786,8 @@ func TestSelector_PrimaryPreferred_ignores_tags(t *testing.T) { func TestSelector_PrimaryPreferred_with_no_primary(t *testing.T) { t.Parallel() - subject := readpref.PrimaryPreferred() + subject, err := readpref.New(readpref.PrimaryPreferredMode, nil) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}). SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -793,12 +800,15 @@ func TestSelector_PrimaryPreferred_with_no_primary(t *testing.T) { func TestSelector_PrimaryPreferred_with_no_primary_and_tags(t *testing.T) { t.Parallel() - subject := readpref.PrimaryPreferred() + rpOpts := &readpref.Options{} tagSet, err := readpref.NewTagSet("a", "2") assert.NoError(t, err) - subject.TagSets = []readpref.TagSet{tagSet} + rpOpts.TagSets = []readpref.TagSet{tagSet} + + subject, err := readpref.New(readpref.PrimaryPreferredMode, rpOpts) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}). SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -811,10 +821,13 @@ func TestSelector_PrimaryPreferred_with_no_primary_and_tags(t *testing.T) { func TestSelector_PrimaryPreferred_with_maxStaleness(t *testing.T) { t.Parallel() - subject := readpref.PrimaryPreferred() + rpOpts := &readpref.Options{} maxStaleness := 90 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, err := readpref.New(readpref.PrimaryPreferredMode, rpOpts) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}). SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -827,10 +840,13 @@ func TestSelector_PrimaryPreferred_with_maxStaleness(t *testing.T) { func TestSelector_PrimaryPreferred_with_maxStaleness_and_no_primary(t *testing.T) { t.Parallel() - subject := readpref.PrimaryPreferred() + rpOpts := &readpref.Options{} maxStaleness := 90 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, err := readpref.New(readpref.PrimaryPreferredMode, rpOpts) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}). SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -843,7 +859,8 @@ func TestSelector_PrimaryPreferred_with_maxStaleness_and_no_primary(t *testing.T func TestSelector_SecondaryPreferred(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred() + subject, err := readpref.New(readpref.SecondaryPreferredMode, nil) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -855,12 +872,15 @@ func TestSelector_SecondaryPreferred(t *testing.T) { func TestSelector_SecondaryPreferred_with_tags(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred() + rpOpts := &readpref.Options{} tagSet, err := readpref.NewTagSet("a", "2") assert.NoError(t, err) - subject.TagSets = []readpref.TagSet{tagSet} + rpOpts.TagSets = []readpref.TagSet{tagSet} + + subject, err := readpref.New(readpref.SecondaryPreferredMode, rpOpts) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -872,12 +892,15 @@ func TestSelector_SecondaryPreferred_with_tags(t *testing.T) { func TestSelector_SecondaryPreferred_with_tags_that_do_not_match(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred() + rpOpts := &readpref.Options{} tagSet, err := readpref.NewTagSet("a", "3") assert.NoError(t, err) - subject.TagSets = []readpref.TagSet{tagSet} + rpOpts.TagSets = []readpref.TagSet{tagSet} + + subject, err := readpref.New(readpref.SecondaryPreferredMode, rpOpts) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -889,12 +912,15 @@ func TestSelector_SecondaryPreferred_with_tags_that_do_not_match(t *testing.T) { func TestSelector_SecondaryPreferred_with_tags_that_do_not_match_and_no_primary(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred() + rpOpts := &readpref.Options{} tagSet, err := readpref.NewTagSet("a", "3") assert.NoError(t, err) - subject.TagSets = []readpref.TagSet{tagSet} + rpOpts.TagSets = []readpref.TagSet{tagSet} + + subject, err := readpref.New(readpref.SecondaryPreferredMode, rpOpts) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -905,7 +931,8 @@ func TestSelector_SecondaryPreferred_with_tags_that_do_not_match_and_no_primary( func TestSelector_SecondaryPreferred_with_no_secondaries(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred() + subject, err := readpref.New(readpref.SecondaryPreferredMode, nil) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestPrimary}) @@ -917,7 +944,7 @@ func TestSelector_SecondaryPreferred_with_no_secondaries(t *testing.T) { func TestSelector_SecondaryPreferred_with_no_secondaries_or_primary(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred() + subject := &readpref.ReadPref{Mode: readpref.SecondaryPreferredMode} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{}) @@ -928,10 +955,13 @@ func TestSelector_SecondaryPreferred_with_no_secondaries_or_primary(t *testing.T func TestSelector_SecondaryPreferred_with_maxStaleness(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred() + rpOpts := &readpref.Options{} maxStaleness := 90 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, err := readpref.New(readpref.SecondaryPreferredMode, rpOpts) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -943,10 +973,13 @@ func TestSelector_SecondaryPreferred_with_maxStaleness(t *testing.T) { func TestSelector_SecondaryPreferred_with_maxStaleness_and_no_primary(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred() + rpOpts := &readpref.Options{} maxStaleness := 90 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, err := readpref.New(readpref.SecondaryPreferredMode, rpOpts) + assert.NoError(t, err) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -958,7 +991,7 @@ func TestSelector_SecondaryPreferred_with_maxStaleness_and_no_primary(t *testing func TestSelector_Secondary(t *testing.T) { t.Parallel() - subject := readpref.Secondary() + subject := &readpref.ReadPref{Mode: readpref.SecondaryPreferredMode} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -970,10 +1003,12 @@ func TestSelector_Secondary(t *testing.T) { func TestSelector_Secondary_with_tags(t *testing.T) { t.Parallel() - subject := readpref.SecondaryPreferred() + rpOpts := &readpref.Options{} maxStaleness := 90 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, _ := readpref.New(readpref.SecondaryMode, rpOpts) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1010,8 +1045,10 @@ func TestSelector_Secondary_with_empty_tag_set(t *testing.T) { } emptyTagSet := readpref.TagSet{} - rp := readpref.SecondaryPreferred() - rp.TagSets = []readpref.TagSet{nonMatchingSet, emptyTagSet} + rpOpts := &readpref.Options{} + rpOpts.TagSets = []readpref.TagSet{nonMatchingSet, emptyTagSet} + + rp, _ := readpref.New(readpref.SecondaryPreferredMode, rpOpts) result, err := (&ReadPref{ReadPref: rp}).SelectServer(topologyNoTags, topologyNoTags.Servers) assert.Nil(t, err, "SelectServer error: %v", err) @@ -1022,12 +1059,13 @@ func TestSelector_Secondary_with_empty_tag_set(t *testing.T) { func TestSelector_Secondary_with_tags_that_do_not_match(t *testing.T) { t.Parallel() - subject := readpref.Secondary() - tagSet, err := readpref.NewTagSet("a", "3") assert.NoError(t, err) - subject.TagSets = []readpref.TagSet{tagSet} + rpOpts := &readpref.Options{} + rpOpts.TagSets = []readpref.TagSet{tagSet} + + subject, _ := readpref.New(readpref.SecondaryMode, rpOpts) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1038,7 +1076,7 @@ func TestSelector_Secondary_with_tags_that_do_not_match(t *testing.T) { func TestSelector_Secondary_with_no_secondaries(t *testing.T) { t.Parallel() - subject := readpref.Secondary() + subject := &readpref.ReadPref{Mode: readpref.SecondaryMode} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestPrimary}) @@ -1049,10 +1087,12 @@ func TestSelector_Secondary_with_no_secondaries(t *testing.T) { func TestSelector_Secondary_with_maxStaleness(t *testing.T) { t.Parallel() - subject := readpref.Secondary() + rpOpts := &readpref.Options{} maxStaleness := 90 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, _ := readpref.New(readpref.SecondaryMode, rpOpts) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1064,10 +1104,12 @@ func TestSelector_Secondary_with_maxStaleness(t *testing.T) { func TestSelector_Secondary_with_maxStaleness_and_no_primary(t *testing.T) { t.Parallel() - subject := readpref.Secondary() + rpOpts := &readpref.Options{} maxStaleness := 90 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, _ := readpref.New(readpref.SecondaryMode, rpOpts) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -1079,7 +1121,7 @@ func TestSelector_Secondary_with_maxStaleness_and_no_primary(t *testing.T) { func TestSelector_Nearest(t *testing.T) { t.Parallel() - subject := readpref.Nearest() + subject := &readpref.ReadPref{Mode: readpref.NearestMode} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1091,12 +1133,14 @@ func TestSelector_Nearest(t *testing.T) { func TestSelector_Nearest_with_tags(t *testing.T) { t.Parallel() - subject := readpref.Nearest() + rpOpts := &readpref.Options{} tagSet, err := readpref.NewTagSet("a", "1") assert.NoError(t, err) - subject.TagSets = []readpref.TagSet{tagSet} + rpOpts.TagSets = []readpref.TagSet{tagSet} + + subject, _ := readpref.New(readpref.NearestMode, rpOpts) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1108,12 +1152,14 @@ func TestSelector_Nearest_with_tags(t *testing.T) { func TestSelector_Nearest_with_tags_that_do_not_match(t *testing.T) { t.Parallel() - subject := readpref.Nearest() + rpOpts := &readpref.Options{} tagSet, err := readpref.NewTagSet("a", "3") assert.NoError(t, err) - subject.TagSets = []readpref.TagSet{tagSet} + rpOpts.TagSets = []readpref.TagSet{tagSet} + + subject, _ := readpref.New(readpref.NearestMode, rpOpts) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1124,7 +1170,7 @@ func TestSelector_Nearest_with_tags_that_do_not_match(t *testing.T) { func TestSelector_Nearest_with_no_primary(t *testing.T) { t.Parallel() - subject := readpref.Nearest() + subject := &readpref.ReadPref{Mode: readpref.NearestMode} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -1136,7 +1182,7 @@ func TestSelector_Nearest_with_no_primary(t *testing.T) { func TestSelector_Nearest_with_no_secondaries(t *testing.T) { t.Parallel() - subject := readpref.Nearest() + subject := &readpref.ReadPref{Mode: readpref.NearestMode} result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestPrimary}) @@ -1148,10 +1194,12 @@ func TestSelector_Nearest_with_no_secondaries(t *testing.T) { func TestSelector_Nearest_with_maxStaleness(t *testing.T) { t.Parallel() - subject := readpref.Nearest() + rpOpts := &readpref.Options{} maxStaleness := 90 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, _ := readpref.New(readpref.NearestMode, rpOpts) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, readPrefTestTopology.Servers) @@ -1163,10 +1211,12 @@ func TestSelector_Nearest_with_maxStaleness(t *testing.T) { func TestSelector_Nearest_with_maxStaleness_and_no_primary(t *testing.T) { t.Parallel() - subject := readpref.Nearest() + rpOpts := &readpref.Options{} maxStaleness := 90 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, _ := readpref.New(readpref.NearestMode, rpOpts) result, err := (&ReadPref{ReadPref: subject}).SelectServer(readPrefTestTopology, []description.Server{readPrefTestSecondary1, readPrefTestSecondary2}) @@ -1178,10 +1228,12 @@ func TestSelector_Nearest_with_maxStaleness_and_no_primary(t *testing.T) { func TestSelector_Max_staleness_is_less_than_90_seconds(t *testing.T) { t.Parallel() - subject := readpref.Nearest() + rpOpts := &readpref.Options{} maxStaleness := 50 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, _ := readpref.New(readpref.NearestMode, rpOpts) s := description.Server{ Addr: address.Address("localhost:27017"), @@ -1204,10 +1256,12 @@ func TestSelector_Max_staleness_is_less_than_90_seconds(t *testing.T) { func TestSelector_Max_staleness_is_too_low(t *testing.T) { t.Parallel() - subject := readpref.Nearest() + rpOpts := &readpref.Options{} maxStaleness := 100 * time.Second - subject.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + subject, _ := readpref.New(readpref.NearestMode, rpOpts) s := description.Server{ Addr: address.Address("localhost:27017"), diff --git a/mongo/client.go b/mongo/client.go index d3e00bef17..1006a40d17 100644 --- a/mongo/client.go +++ b/mongo/client.go @@ -446,9 +446,11 @@ func (c *Client) endSessions(ctx context.Context) { return } + rpOpts, _ := readpref.New(readpref.PrimaryPreferredMode, nil) sessionIDs := c.sessionPool.IDSlice() + op := operation.NewEndSessions(nil).ClusterClock(c.clock).Deployment(c.deployment). - ServerSelector(&serverselector.ReadPref{ReadPref: readpref.PrimaryPreferred()}). + ServerSelector(&serverselector.ReadPref{ReadPref: rpOpts}). CommandMonitor(c.monitor).Database("admin").Crypt(c.cryptFLE).ServerAPI(c.serverAPI) totalNumIDs := len(sessionIDs) diff --git a/mongo/client_test.go b/mongo/client_test.go index 44d7c13aec..aba0e551d1 100644 --- a/mongo/client_test.go +++ b/mongo/client_test.go @@ -117,7 +117,7 @@ func TestClient(t *testing.T) { assert.Equal(t, gotMode, readpref.SecondaryMode, "expected mode %v, got %v", readpref.SecondaryMode, gotMode) gotTags := client.readPreference.TagSets assert.Equal(t, gotTags, tags, "expected tags %v, got %v", tags, gotTags) - gotStaleness := client.readPreference.MaxStaleness + gotStaleness := client.readPreference.MaxStaleness() require.NotNil(t, gotStaleness, "expected max staleness to be set but was not") wantStaleness := time.Duration(5) * time.Second assert.Equal(t, *gotStaleness, wantStaleness, "expected staleness %v, got %v", wantStaleness, *gotStaleness) diff --git a/mongo/collection_test.go b/mongo/collection_test.go index ffff38753d..ec7b7bb88a 100644 --- a/mongo/collection_test.go +++ b/mongo/collection_test.go @@ -45,7 +45,7 @@ func TestCollection(t *testing.T) { }) t.Run("specified options", func(t *testing.T) { rpPrimary := readpref.Primary() - rpSecondary := readpref.Secondary() + rpSecondary := &readpref.ReadPref{Mode: readpref.SecondaryMode} wc1 := &writeconcern.WriteConcern{W: 5} wc2 := &writeconcern.WriteConcern{W: 10} rcLocal := readconcern.Local() diff --git a/mongo/crud_examples_test.go b/mongo/crud_examples_test.go index 658ff2451c..9676a7c3eb 100644 --- a/mongo/crud_examples_test.go +++ b/mongo/crud_examples_test.go @@ -755,8 +755,9 @@ func ExampleClient_StartSession_withTransaction() { // Specify the ReadPreference option to set the read preference to primary // preferred for this transaction. - txnOpts := options.Transaction(). - SetReadPreference(readpref.PrimaryPreferred()) + rp := &readpref.ReadPref{Mode: readpref.PrimaryPreferredMode} + txnOpts := options.Transaction().SetReadPreference(rp) + result, err := sess.WithTransaction( context.TODO(), func(ctx context.Context) (interface{}, error) { diff --git a/mongo/database.go b/mongo/database.go index a782454425..4bf0f341eb 100644 --- a/mongo/database.go +++ b/mongo/database.go @@ -183,7 +183,9 @@ func (db *Database) processRunCommand(ctx context.Context, cmd interface{}, ro.ReadPreference = opt.ReadPreference } } - if sess != nil && sess.TransactionRunning() && ro.ReadPreference != nil && ro.ReadPreference.Mode != readpref.PrimaryMode { + if sess != nil && sess.TransactionRunning() && ro.ReadPreference != nil && + ro.ReadPreference.Mode != readpref.PrimaryMode { + return nil, sess, errors.New("read preference in a transaction must be primary") } diff --git a/mongo/database_test.go b/mongo/database_test.go index 31bd900439..36d8ceb0d8 100644 --- a/mongo/database_test.go +++ b/mongo/database_test.go @@ -48,7 +48,7 @@ func TestDatabase(t *testing.T) { t.Run("options", func(t *testing.T) { t.Run("custom", func(t *testing.T) { rpPrimary := readpref.Primary() - rpSecondary := readpref.Secondary() + rpSecondary := &readpref.ReadPref{Mode: readpref.SecondaryMode} wc1 := &writeconcern.WriteConcern{W: 5} wc2 := &writeconcern.WriteConcern{W: 10} rcLocal := readconcern.Local() diff --git a/mongo/options/clientoptions.go b/mongo/options/clientoptions.go index e261008f3f..cbd6815336 100644 --- a/mongo/options/clientoptions.go +++ b/mongo/options/clientoptions.go @@ -431,23 +431,29 @@ func (c *ClientOptions) ApplyURI(uri string) *ClientOptions { } if cs.ReadPreference != "" || len(cs.ReadPreferenceTagSets) > 0 || cs.MaxStalenessSet { + readprefOpts := &readpref.Options{} + + tagSets := readpref.NewTagSetsFromMaps(cs.ReadPreferenceTagSets) + if len(tagSets) > 0 { + readprefOpts.TagSets = tagSets + } + + if cs.MaxStaleness != 0 { + readprefOpts.MaxStaleness = &cs.MaxStaleness + } + mode, err := readpref.ModeFromString(cs.ReadPreference) if err != nil { c.err = err - return c - } - c.ReadPreference = &readpref.ReadPref{ - Mode: mode, + return c } - tagSets := readpref.NewTagSetsFromMaps(cs.ReadPreferenceTagSets) - if len(tagSets) > 0 { - c.ReadPreference.TagSets = tagSets - } + c.ReadPreference, err = readpref.New(mode, readprefOpts) + if err != nil { + c.err = err - if cs.MaxStaleness != 0 { - c.ReadPreference.MaxStaleness = &cs.MaxStaleness + return c } } diff --git a/mongo/options/clientoptions_test.go b/mongo/options/clientoptions_test.go index c17fb28660..dba04dbce3 100644 --- a/mongo/options/clientoptions_test.go +++ b/mongo/options/clientoptions_test.go @@ -79,7 +79,7 @@ func TestClientOptions(t *testing.T) { {"PoolMonitor", (*ClientOptions).SetPoolMonitor, &event.PoolMonitor{}, "PoolMonitor", false}, {"Monitor", (*ClientOptions).SetMonitor, &event.CommandMonitor{}, "Monitor", false}, {"ReadConcern", (*ClientOptions).SetReadConcern, readconcern.Majority(), "ReadConcern", false}, - {"ReadPreference", (*ClientOptions).SetReadPreference, readpref.SecondaryPreferred(), "ReadPreference", false}, + {"ReadPreference", (*ClientOptions).SetReadPreference, &readpref.ReadPref{Mode: readpref.SecondaryPreferredMode}, "ReadPreference", false}, {"Registry", (*ClientOptions).SetRegistry, bson.NewRegistry(), "Registry", false}, {"ReplicaSet", (*ClientOptions).SetReplicaSet, "example-replicaset", "ReplicaSet", true}, {"RetryWrites", (*ClientOptions).SetRetryWrites, true, "RetryWrites", true}, @@ -353,16 +353,17 @@ func TestClientOptions(t *testing.T) { { "ReadPreference", "mongodb://localhost/?readPreference=secondaryPreferred", - baseClient().SetReadPreference(readpref.SecondaryPreferred()), + baseClient().SetReadPreference(&readpref.ReadPref{Mode: readpref.SecondaryPreferredMode}), }, { "ReadPreferenceTagSets", "mongodb://localhost/?readPreference=secondaryPreferred&readPreferenceTags=foo:bar", baseClient().SetReadPreference(func() *readpref.ReadPref { - rp := readpref.SecondaryPreferred() //readpref.NewTagSet("foo", "bar") - tagSet, _ := readpref.NewTagSet("foo", "bar") - rp.TagSets = append(rp.TagSets, tagSet) + + rp, _ := readpref.New(readpref.SecondaryPreferredMode, &readpref.Options{ + TagSets: []readpref.TagSet{tagSet}, + }) return rp }()), @@ -371,10 +372,11 @@ func TestClientOptions(t *testing.T) { "MaxStaleness", "mongodb://localhost/?readPreference=secondaryPreferred&maxStaleness=250", baseClient().SetReadPreference(func() *readpref.ReadPref { - rp := readpref.SecondaryPreferred() - maxStaleness := 250 * time.Second - rp.MaxStaleness = &maxStaleness + + rp, _ := readpref.New(readpref.SecondaryPreferredMode, &readpref.Options{ + MaxStaleness: &maxStaleness, + }) return rp }()), diff --git a/mongo/readpref/readpref.go b/mongo/readpref/readpref.go index 58409a4d58..3ed062f673 100644 --- a/mongo/readpref/readpref.go +++ b/mongo/readpref/readpref.go @@ -9,49 +9,75 @@ package readpref import ( "bytes" + "errors" "fmt" "time" ) -// Primary constructs a read preference with a PrimaryMode. -func Primary() *ReadPref { - return &ReadPref{Mode: PrimaryMode} -} +var errInvalidReadPreference = errors.New("can not specify tags, max staleness, or hedge with mode primary") -// PrimaryPreferred constructs a read preference with a PrimaryPreferredMode. -func PrimaryPreferred() *ReadPref { - return &ReadPref{Mode: PrimaryPreferredMode} -} +// Options defines the options for constructing a read preference. +type Options struct { + // Maximum amount of time to allow a server to be considered eligible for + // selection. The second return value indicates if this value has been set. + MaxStaleness *time.Duration -// SecondaryPreferred constructs a read preference with a SecondaryPreferredMode. -func SecondaryPreferred() *ReadPref { - return &ReadPref{Mode: SecondaryPreferredMode} -} + // Tag sets indicating which servers should be considered. + TagSets []TagSet -// Secondary constructs a read preference with a SecondaryMode. -func Secondary() *ReadPref { - return &ReadPref{Mode: SecondaryMode} + // Specify whether or not hedged reads are enabled for this read preference. + HedgeEnabled *bool } -// Nearest constructs a read preference with a NearestMode. -func Nearest() *ReadPref { - return &ReadPref{Mode: NearestMode} +func optionsExist(opts *Options) bool { + return opts != nil && (opts.MaxStaleness != nil || len(opts.TagSets) > 0 || opts.HedgeEnabled != nil) } // ReadPref determines which servers are considered suitable for read operations. type ReadPref struct { - // Maximum amount of time to allow a server to be considered eligible for - // selection. The second return value indicates if this value has been set. - MaxStaleness *time.Duration - - // Mode indicates the mode of the read preference. Mode Mode + opts Options +} - // Tag sets indicating which servers should be considered. - TagSets []TagSet +// New creates a new ReadPref. +func New(mode Mode, opts *Options) (*ReadPref, error) { + if mode == PrimaryMode && optionsExist(opts) { + return nil, errInvalidReadPreference + } - // Specify whether or not hedged reads are enabled for this read preference. - HedgeEnabled *bool + rp := &ReadPref{ + Mode: mode, + } + + if opts != nil { + rp.opts = *opts + } + + return rp, nil +} + +// Primary constructs a read preference with a PrimaryMode. +func Primary() *ReadPref { + return &ReadPref{Mode: PrimaryMode} +} + +// MaxStaleness is the maximum amount of time to allow +// a server to be considered eligible for selection. The +// second return value indicates if this value has been set. +func (r *ReadPref) MaxStaleness() *time.Duration { + return r.opts.MaxStaleness +} + +// TagSets are multiple tag sets indicating +// which servers should be considered. +func (r *ReadPref) TagSets() []TagSet { + return r.opts.TagSets +} + +// HedgeEnabled returns whether or not hedged reads are enabled for this read preference. If this option was not +// specified during read preference construction, nil is returned. +func (r *ReadPref) HedgeEnabled() *bool { + return r.opts.HedgeEnabled } // String returns a human-readable description of the read preference. @@ -59,16 +85,16 @@ func (r *ReadPref) String() string { var b bytes.Buffer b.WriteString(r.Mode.String()) delim := "(" - if r.MaxStaleness != nil { - fmt.Fprintf(&b, "%smaxStaleness=%v", delim, r.MaxStaleness) + if r.MaxStaleness() != nil { + fmt.Fprintf(&b, "%smaxStaleness=%v", delim, r.MaxStaleness()) delim = " " } - for _, tagSet := range r.TagSets { + for _, tagSet := range r.TagSets() { fmt.Fprintf(&b, "%stagSet=%s", delim, tagSet.String()) delim = " " } - if r.HedgeEnabled != nil { - fmt.Fprintf(&b, "%shedgeEnabled=%v", delim, *r.HedgeEnabled) + if r.HedgeEnabled() != nil { + fmt.Fprintf(&b, "%shedgeEnabled=%v", delim, *r.HedgeEnabled()) delim = " " } if delim != "(" { diff --git a/mongo/readpref/readpref_test.go b/mongo/readpref/readpref_test.go index 73c1d29ec6..8a082e74d8 100644 --- a/mongo/readpref/readpref_test.go +++ b/mongo/readpref/readpref_test.go @@ -15,26 +15,32 @@ import ( func TestReadPref_String(t *testing.T) { t.Run("ReadPref.String() with all options", func(t *testing.T) { - readPref := Nearest() + rpOpts := &Options{} maxStaleness := 120 * time.Second - readPref.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness - readPref.TagSets = []TagSet{{{"a", "1"}, {"b", "2"}}, {{"q", "5"}, {"r", "6"}}} + rpOpts.TagSets = []TagSet{{{"a", "1"}, {"b", "2"}}, {{"q", "5"}, {"r", "6"}}} hedgeEnabled := true - readPref.HedgeEnabled = &hedgeEnabled + rpOpts.HedgeEnabled = &hedgeEnabled + + readPref, err := New(NearestMode, rpOpts) + assert.NoError(t, err) expected := "nearest(maxStaleness=2m0s tagSet=a=1,b=2 tagSet=q=5,r=6 hedgeEnabled=true)" assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) }) t.Run("ReadPref.String() with one option", func(t *testing.T) { - readPref := Secondary() + rpOpts := &Options{} tagSet, err := NewTagSet("a", "1", "b", "2") assert.NoError(t, err) - readPref.TagSets = append(readPref.TagSets, tagSet) + rpOpts.TagSets = []TagSet{tagSet} + + readPref, err := New(SecondaryMode, rpOpts) + assert.NoError(t, err) expected := "secondary(tagSet=a=1,b=2)" assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index 902008770b..c0bc073e3c 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -1764,9 +1764,9 @@ func (op Operation) createReadPref(desc description.SelectedServer, isOpQuery bo case readpref.PrimaryPreferredMode: doc = bsoncore.AppendStringElement(doc, "mode", "primaryPreferred") case readpref.SecondaryPreferredMode: - ok := rp.MaxStaleness != nil - if desc.Server.Kind == description.ServerKindMongos && isOpQuery && !ok && len(rp.TagSets) == 0 && - rp.HedgeEnabled == nil { + ok := rp.MaxStaleness() != nil + if desc.Server.Kind == description.ServerKindMongos && isOpQuery && !ok && len(rp.TagSets()) == 0 && + rp.HedgeEnabled() == nil { return nil, nil } @@ -1777,8 +1777,8 @@ func (op Operation) createReadPref(desc description.SelectedServer, isOpQuery bo doc = bsoncore.AppendStringElement(doc, "mode", "nearest") } - sets := make([]bsoncore.Document, 0, len(rp.TagSets)) - for _, ts := range rp.TagSets { + sets := make([]bsoncore.Document, 0, len(rp.TagSets())) + for _, ts := range rp.TagSets() { i, set := bsoncore.AppendDocumentStart(nil) for _, t := range ts { set = bsoncore.AppendStringElement(set, t.Name, t.Value) @@ -1795,11 +1795,11 @@ func (op Operation) createReadPref(desc description.SelectedServer, isOpQuery bo doc, _ = bsoncore.AppendArrayEnd(doc, aidx) } - if maxStaleness := rp.MaxStaleness; maxStaleness != nil { + if maxStaleness := rp.MaxStaleness(); maxStaleness != nil { doc = bsoncore.AppendInt32Element(doc, "maxStalenessSeconds", int32((*maxStaleness).Seconds())) } - if hedgeEnabled := rp.HedgeEnabled; hedgeEnabled != nil { + if hedgeEnabled := rp.HedgeEnabled(); hedgeEnabled != nil { var hedgeIdx int32 hedgeIdx, doc = bsoncore.AppendDocumentElementStart(doc, "hedge") doc = bsoncore.AppendBooleanElement(doc, "enabled", *hedgeEnabled) diff --git a/x/mongo/driver/operation_test.go b/x/mongo/driver/operation_test.go index 4de77cc45b..29e3eba9b8 100644 --- a/x/mongo/driver/operation_test.go +++ b/x/mongo/driver/operation_test.go @@ -437,20 +437,22 @@ func TestOperation(t *testing.T) { {"primary/mongos", readpref.Primary(), description.ServerKindMongos, description.TopologyKindSharded, false, nil}, {"primary/single", readpref.Primary(), description.ServerKindRSPrimary, description.TopologyKindSingle, false, rpPrimaryPreferred}, {"primary/primary", readpref.Primary(), description.ServerKindRSPrimary, description.TopologyKindReplicaSet, false, nil}, - {"primaryPreferred", readpref.PrimaryPreferred(), description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpPrimaryPreferred}, - {"secondaryPreferred/mongos/opquery", readpref.SecondaryPreferred(), description.ServerKindMongos, description.TopologyKindSharded, true, nil}, - {"secondaryPreferred", readpref.SecondaryPreferred(), description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpSecondaryPreferred}, - {"secondary", readpref.Secondary(), description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpSecondary}, - {"nearest", readpref.Nearest(), description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpNearest}, + {"primaryPreferred", &readpref.ReadPref{Mode: readpref.PrimaryPreferredMode}, description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpPrimaryPreferred}, + {"secondaryPreferred/mongos/opquery", &readpref.ReadPref{Mode: readpref.SecondaryMode}, description.ServerKindMongos, description.TopologyKindSharded, true, nil}, + {"secondaryPreferred", &readpref.ReadPref{Mode: readpref.SecondaryMode}, description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpSecondaryPreferred}, + {"secondary", &readpref.ReadPref{Mode: readpref.SecondaryMode}, description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpSecondary}, + {"nearest", &readpref.ReadPref{Mode: readpref.NearestMode}, description.ServerKindRSSecondary, description.TopologyKindReplicaSet, false, rpNearest}, { "secondaryPreferred/withTags", func() *readpref.ReadPref { - rp := readpref.SecondaryPreferred() + rpOpts := &readpref.Options{} tagSet, err := readpref.NewTagSet("disk", "ssd", "use", "reporting") assert.NoError(t, err) - rp.TagSets = []readpref.TagSet{tagSet} + rpOpts.TagSets = []readpref.TagSet{tagSet} + + rp, _ := readpref.New(readpref.SecondaryPreferredMode, rpOpts) return rp }(), @@ -462,12 +464,15 @@ func TestOperation(t *testing.T) { { "secondaryPreferred/withTags/emptyTagSet", func() *readpref.ReadPref { - rp := readpref.SecondaryPreferred() - rp.TagSets = []readpref.TagSet{ + rpOpts := &readpref.Options{} + + rpOpts.TagSets = []readpref.TagSet{ readpref.TagSet{{Name: "disk", Value: "ssd"}}, readpref.TagSet{}, } + rp, _ := readpref.New(readpref.SecondaryPreferredMode, rpOpts) + return rp }(), description.ServerKindRSSecondary, @@ -484,10 +489,12 @@ func TestOperation(t *testing.T) { { "secondaryPreferred/withMaxStaleness", func() *readpref.ReadPref { - rp := readpref.SecondaryPreferred() + rpOpts := &readpref.Options{} maxStaleness := 25 * time.Second - rp.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness + + rp, _ := readpref.New(readpref.SecondaryPreferredMode, rpOpts) return rp }(), @@ -497,10 +504,12 @@ func TestOperation(t *testing.T) { // A read preference document is generated for SecondaryPreferred if the hedge document is non-nil. "secondaryPreferred with hedge to mongos using OP_QUERY", func() *readpref.ReadPref { - rp := readpref.SecondaryPreferred() + rpOpts := &readpref.Options{} he := true - rp.HedgeEnabled = &he + rpOpts.HedgeEnabled = &he + + rp, _ := readpref.New(readpref.SecondaryPreferredMode, rpOpts) return rp }(), @@ -512,18 +521,20 @@ func TestOperation(t *testing.T) { { "secondaryPreferred with all options", func() *readpref.ReadPref { - rp := readpref.SecondaryPreferred() - tagSet, err := readpref.NewTagSet("disk", "ssd", "use", "reporting") assert.NoError(t, err) - rp.TagSets = []readpref.TagSet{tagSet} + rpOpts := &readpref.Options{} + + rpOpts.TagSets = []readpref.TagSet{tagSet} maxStaleness := 25 * time.Second - rp.MaxStaleness = &maxStaleness + rpOpts.MaxStaleness = &maxStaleness he := false - rp.HedgeEnabled = &he + rpOpts.HedgeEnabled = &he + + rp, _ := readpref.New(readpref.SecondaryPreferredMode, rpOpts) return rp }(), @@ -562,7 +573,7 @@ func TestOperation(t *testing.T) { }) t.Run("readPreference", func(t *testing.T) { want := wiremessage.SecondaryOK - got := Operation{ReadPreference: readpref.Secondary()}.secondaryOK(description.SelectedServer{}) + got := Operation{ReadPreference: &readpref.ReadPref{Mode: readpref.SecondaryMode}}.secondaryOK(description.SelectedServer{}) if got != want { t.Errorf("Did not receive expected query flags. got %v; want %v", got, want) } diff --git a/x/mongo/driver/topology/topology_test.go b/x/mongo/driver/topology/topology_test.go index 0e4920d88b..9d3deefe24 100644 --- a/x/mongo/driver/topology/topology_test.go +++ b/x/mongo/driver/topology/topology_test.go @@ -888,7 +888,7 @@ func runInWindowTest(t *testing.T, directory string, filename string) { for i := 0; i < test.Iterations; i++ { selected, err := topology.SelectServer( context.Background(), - &serverselector.ReadPref{ReadPref: readpref.Nearest()}) + &serverselector.ReadPref{ReadPref: &readpref.ReadPref{Mode: readpref.NearestMode}}) require.NoError(t, err, "error selecting server") counts[string(selected.(*SelectedServer).address)]++ } From e1f9dc66cc483890de5a8ae35120453a08dcf934 Mon Sep 17 00:00:00 2001 From: Preston Vasquez Date: Mon, 5 Aug 2024 11:47:11 -0600 Subject: [PATCH 3/4] GODRIVER-2689 Resolve merge conflicts --- mongo/readpref/readpref.go | 126 +++++++++++++----- mongo/readpref/readpref_test.go | 220 +++++++++++++++++++++++++++----- 2 files changed, 285 insertions(+), 61 deletions(-) diff --git a/mongo/readpref/readpref.go b/mongo/readpref/readpref.go index 3ed062f673..59104c771d 100644 --- a/mongo/readpref/readpref.go +++ b/mongo/readpref/readpref.go @@ -16,44 +16,72 @@ import ( var errInvalidReadPreference = errors.New("can not specify tags, max staleness, or hedge with mode primary") -// Options defines the options for constructing a read preference. -type Options struct { - // Maximum amount of time to allow a server to be considered eligible for - // selection. The second return value indicates if this value has been set. - MaxStaleness *time.Duration +// ReadPref determines which servers are considered suitable for read operations. +type ReadPref struct { + Mode Mode - // Tag sets indicating which servers should be considered. - TagSets []TagSet + maxStaleness *time.Duration + tagSets []TagSet + hedgeEnabled *bool +} - // Specify whether or not hedged reads are enabled for this read preference. - HedgeEnabled *bool +type ReadPrefBuilder struct { + opts []func(*ReadPref) } -func optionsExist(opts *Options) bool { - return opts != nil && (opts.MaxStaleness != nil || len(opts.TagSets) > 0 || opts.HedgeEnabled != nil) +func Options() *ReadPrefBuilder { + return &ReadPrefBuilder{} } -// ReadPref determines which servers are considered suitable for read operations. -type ReadPref struct { - Mode Mode - opts Options +func (bldr *ReadPrefBuilder) List() []func(*ReadPref) { + return bldr.opts } -// New creates a new ReadPref. -func New(mode Mode, opts *Options) (*ReadPref, error) { - if mode == PrimaryMode && optionsExist(opts) { - return nil, errInvalidReadPreference - } +func (bldr *ReadPrefBuilder) SetMaxStaleness(dur time.Duration) *ReadPrefBuilder { + bldr.opts = append(bldr.opts, func(opts *ReadPref) { + opts.maxStaleness = &dur + }) - rp := &ReadPref{ - Mode: mode, + return bldr +} + +func (bldr *ReadPrefBuilder) SetTagSets(sets []TagSet) *ReadPrefBuilder { + bldr.opts = append(bldr.opts, func(opts *ReadPref) { + opts.tagSets = sets + }) + + return bldr +} + +func (bldr *ReadPrefBuilder) SetHedgeEnabled(hedgeEnabled bool) *ReadPrefBuilder { + bldr.opts = append(bldr.opts, func(opts *ReadPref) { + opts.hedgeEnabled = &hedgeEnabled + }) + + return bldr +} + +func validOpts(mode Mode, opts *ReadPref) bool { + if opts == nil || mode != PrimaryMode { + return true } - if opts != nil { - rp.opts = *opts + return opts.maxStaleness == nil && len(opts.tagSets) == 0 && opts.hedgeEnabled == nil +} + +func mergeBuilders(builders ...*ReadPrefBuilder) *ReadPref { + opts := new(ReadPref) + for _, bldr := range builders { + if bldr == nil { + continue + } + + for _, setterFn := range bldr.List() { + setterFn(opts) + } } - return rp, nil + return opts } // Primary constructs a read preference with a PrimaryMode. @@ -61,23 +89,63 @@ func Primary() *ReadPref { return &ReadPref{Mode: PrimaryMode} } +// PrimaryPreferred constructs a read preference with a PrimaryPreferredMode. +func PrimaryPreferred(opts ...*ReadPrefBuilder) *ReadPref { + // New only returns an error with a mode of Primary + rp, _ := New(PrimaryPreferredMode, opts...) + return rp +} + +// SecondaryPreferred constructs a read preference with a SecondaryPreferredMode. +func SecondaryPreferred(opts ...*ReadPrefBuilder) *ReadPref { + // New only returns an error with a mode of Primary + rp, _ := New(SecondaryPreferredMode, opts...) + return rp +} + +// Secondary constructs a read preference with a SecondaryMode. +func Secondary(opts ...*ReadPrefBuilder) *ReadPref { + // New only returns an error with a mode of Primary + rp, _ := New(SecondaryMode, opts...) + return rp +} + +// Nearest constructs a read preference with a NearestMode. +func Nearest(opts ...*ReadPrefBuilder) *ReadPref { + // New only returns an error with a mode of Primary + rp, _ := New(NearestMode, opts...) + return rp +} + +// New creates a new ReadPref. +func New(mode Mode, builders ...*ReadPrefBuilder) (*ReadPref, error) { + rp := mergeBuilders(builders...) + rp.Mode = mode + + if !validOpts(mode, rp) { + return nil, errInvalidReadPreference + } + + return rp, nil +} + // MaxStaleness is the maximum amount of time to allow // a server to be considered eligible for selection. The // second return value indicates if this value has been set. func (r *ReadPref) MaxStaleness() *time.Duration { - return r.opts.MaxStaleness + return r.maxStaleness } // TagSets are multiple tag sets indicating // which servers should be considered. func (r *ReadPref) TagSets() []TagSet { - return r.opts.TagSets + return r.tagSets } // HedgeEnabled returns whether or not hedged reads are enabled for this read preference. If this option was not // specified during read preference construction, nil is returned. func (r *ReadPref) HedgeEnabled() *bool { - return r.opts.HedgeEnabled + return r.hedgeEnabled } // String returns a human-readable description of the read preference. @@ -86,7 +154,7 @@ func (r *ReadPref) String() string { b.WriteString(r.Mode.String()) delim := "(" if r.MaxStaleness() != nil { - fmt.Fprintf(&b, "%smaxStaleness=%v", delim, r.MaxStaleness()) + fmt.Fprintf(&b, "%smaxStaleness=%v", delim, *r.MaxStaleness()) delim = " " } for _, tagSet := range r.TagSets() { diff --git a/mongo/readpref/readpref_test.go b/mongo/readpref/readpref_test.go index 8a082e74d8..a76336783b 100644 --- a/mongo/readpref/readpref_test.go +++ b/mongo/readpref/readpref_test.go @@ -8,46 +8,202 @@ package readpref import ( "testing" - "time" - "go.mongodb.org/mongo-driver/internal/assert" + "go.mongodb.org/mongo-driver/v2/internal/assert" ) -func TestReadPref_String(t *testing.T) { - t.Run("ReadPref.String() with all options", func(t *testing.T) { - rpOpts := &Options{} +func TestNew(t *testing.T) { + t.Parallel() - maxStaleness := 120 * time.Second - rpOpts.MaxStaleness = &maxStaleness + tests := []struct { + name string + mode Mode + opts []*ReadPrefBuilder + want *ReadPref + wantErr error + }{ + { + name: "primary no options", + mode: PrimaryMode, + opts: nil, + want: &ReadPref{Mode: PrimaryMode}, + wantErr: nil, + }, + { + name: "primary with maxStaleness", + mode: PrimaryMode, + opts: []*ReadPrefBuilder{Options().SetMaxStaleness(1)}, + want: nil, + wantErr: errInvalidReadPreference, + }, + { + name: "primary with tags", + mode: PrimaryMode, + opts: []*ReadPrefBuilder{Options().SetTagSets([]TagSet{{}})}, + want: nil, + wantErr: errInvalidReadPreference, + }, + { + name: "primary with hedgeEnabled", + mode: PrimaryMode, + opts: []*ReadPrefBuilder{Options().SetHedgeEnabled(false)}, + want: nil, + wantErr: errInvalidReadPreference, + }, + { + name: "primaryPreferred", + mode: PrimaryPreferredMode, + opts: nil, + want: &ReadPref{Mode: PrimaryPreferredMode}, + wantErr: nil, + }, + } - rpOpts.TagSets = []TagSet{{{"a", "1"}, {"b", "2"}}, {{"q", "5"}, {"r", "6"}}} + for _, test := range tests { + test := test - hedgeEnabled := true - rpOpts.HedgeEnabled = &hedgeEnabled + t.Run(test.name, func(t *testing.T) { + t.Parallel() - readPref, err := New(NearestMode, rpOpts) - assert.NoError(t, err) + readPref, err := New(test.mode, test.opts...) - expected := "nearest(maxStaleness=2m0s tagSet=a=1,b=2 tagSet=q=5,r=6 hedgeEnabled=true)" - assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) - }) - t.Run("ReadPref.String() with one option", func(t *testing.T) { - rpOpts := &Options{} + if test.wantErr == nil { + assert.NoError(t, err) + } else { + assert.ErrorIs(t, err, test.wantErr) + } - tagSet, err := NewTagSet("a", "1", "b", "2") - assert.NoError(t, err) + if test.want == nil { + return + } - rpOpts.TagSets = []TagSet{tagSet} - - readPref, err := New(SecondaryMode, rpOpts) - assert.NoError(t, err) - - expected := "secondary(tagSet=a=1,b=2)" - assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) - }) - t.Run("ReadPref.String() with no options", func(t *testing.T) { - readPref := Primary() - expected := "primary" - assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) - }) + assert.Equal(t, test.mode, readPref.Mode) + assert.EqualValues(t, test.want, readPref) + }) + } } + +// +//func TestPrimaryPreferred(t *testing.T) { +// subject := PrimaryPreferred() +// +// require.Equal(t, PrimaryPreferredMode, subject.Mode()) +// _, set := subject.MaxStaleness() +// require.False(t, set) +// require.Len(t, subject.TagSets(), 0) +//} +// +//func TestPrimaryPreferred_with_options(t *testing.T) { +// subject := PrimaryPreferred( +// WithMaxStaleness(time.Duration(10)), +// WithTags("a", "1", "b", "2"), +// ) +// +// require.Equal(t, PrimaryPreferredMode, subject.Mode()) +// ms, set := subject.MaxStaleness() +// require.True(t, set) +// require.Equal(t, time.Duration(10), ms) +// require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) +//} +// +//func TestSecondaryPreferred(t *testing.T) { +// subject := SecondaryPreferred() +// +// require.Equal(t, SecondaryPreferredMode, subject.Mode()) +// _, set := subject.MaxStaleness() +// require.False(t, set) +// require.Len(t, subject.TagSets(), 0) +//} +// +//func TestSecondaryPreferred_with_options(t *testing.T) { +// subject := SecondaryPreferred( +// WithMaxStaleness(time.Duration(10)), +// WithTags("a", "1", "b", "2"), +// ) +// +// require.Equal(t, SecondaryPreferredMode, subject.Mode()) +// ms, set := subject.MaxStaleness() +// require.True(t, set) +// require.Equal(t, time.Duration(10), ms) +// require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) +//} +// +//func TestSecondary(t *testing.T) { +// subject := Secondary() +// +// require.Equal(t, SecondaryMode, subject.Mode()) +// _, set := subject.MaxStaleness() +// require.False(t, set) +// require.Len(t, subject.TagSets(), 0) +//} +// +//func TestSecondary_with_options(t *testing.T) { +// subject := Secondary( +// WithMaxStaleness(time.Duration(10)), +// WithTags("a", "1", "b", "2"), +// ) +// +// require.Equal(t, SecondaryMode, subject.Mode()) +// ms, set := subject.MaxStaleness() +// require.True(t, set) +// require.Equal(t, time.Duration(10), ms) +// require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) +//} +// +//func TestNearest(t *testing.T) { +// subject := Nearest() +// +// require.Equal(t, NearestMode, subject.Mode()) +// _, set := subject.MaxStaleness() +// require.False(t, set) +// require.Len(t, subject.TagSets(), 0) +//} +// +//func TestNearest_with_options(t *testing.T) { +// subject := Nearest( +// WithMaxStaleness(time.Duration(10)), +// WithTags("a", "1", "b", "2"), +// ) +// +// require.Equal(t, NearestMode, subject.Mode()) +// ms, set := subject.MaxStaleness() +// require.True(t, set) +// require.Equal(t, time.Duration(10), ms) +// require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) +//} +// +//func TestHedge(t *testing.T) { +// t.Run("hedge specified with primary mode errors", func(t *testing.T) { +// _, err := New(PrimaryMode, WithHedgeEnabled(true)) +// assert.Equal(t, errInvalidReadPreference, err, "expected error %v, got %v", errInvalidReadPreference, err) +// }) +// t.Run("valid hedge document and mode succeeds", func(t *testing.T) { +// rp, err := New(SecondaryMode, WithHedgeEnabled(true)) +// assert.Nil(t, err, "expected no error, got %v", err) +// enabled := rp.HedgeEnabled() +// assert.NotNil(t, enabled, "expected HedgeEnabled to return a non-nil value, got nil") +// assert.True(t, *enabled, "expected HedgeEnabled to return true, got false") +// }) +//} +// +//func TestReadPref_String(t *testing.T) { +// t.Run("ReadPref.String() with all options", func(t *testing.T) { +// readPref := Nearest( +// WithMaxStaleness(120*time.Second), +// WithTagSets(tag.Set{{"a", "1"}, {"b", "2"}}, tag.Set{{"q", "5"}, {"r", "6"}}), +// WithHedgeEnabled(true), +// ) +// expected := "nearest(maxStaleness=2m0s tagSet=a=1,b=2 tagSet=q=5,r=6 hedgeEnabled=true)" +// assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) +// }) +// t.Run("ReadPref.String() with one option", func(t *testing.T) { +// readPref := Secondary(WithTags("a", "1", "b", "2")) +// expected := "secondary(tagSet=a=1,b=2)" +// assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) +// }) +// t.Run("ReadPref.String() with no options", func(t *testing.T) { +// readPref := Primary() +// expected := "primary" +// assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) +// }) +//} From cb94db3f19e353b5534b2857a184de95bfd39d04 Mon Sep 17 00:00:00 2001 From: Preston Vasquez Date: Tue, 6 Aug 2024 13:56:19 -0600 Subject: [PATCH 4/4] GODRIVER-2689 Clean up readpref tests --- mongo/readpref/mode.go | 33 ++--- mongo/readpref/mode_test.go | 35 ------ mongo/readpref/readpref.go | 2 +- mongo/readpref/readpref_test.go | 210 +++++++++++++------------------- 4 files changed, 93 insertions(+), 187 deletions(-) delete mode 100644 mongo/readpref/mode_test.go diff --git a/mongo/readpref/mode.go b/mongo/readpref/mode.go index deacf9f337..aa2c25099c 100644 --- a/mongo/readpref/mode.go +++ b/mongo/readpref/mode.go @@ -12,29 +12,28 @@ import ( ) // Mode indicates the user's preference on reads. -type Mode uint8 +type Mode string // Mode constants const ( - _ Mode = iota // PrimaryMode indicates that only a primary is // considered for reading. This is the default // mode. - PrimaryMode + PrimaryMode = "primary" // PrimaryPreferredMode indicates that if a primary // is available, use it; otherwise, eligible // secondaries will be considered. - PrimaryPreferredMode + PrimaryPreferredMode = "primaryPreferred" // SecondaryMode indicates that only secondaries // should be considered. - SecondaryMode + SecondaryMode = "secondary" // SecondaryPreferredMode indicates that only secondaries // should be considered when one is available. If none // are available, then a primary will be considered. - SecondaryPreferredMode + SecondaryPreferredMode = "secondaryPreferred" // NearestMode indicates that all primaries and secondaries // will be considered. - NearestMode + NearestMode = "nearest" ) // ModeFromString returns a mode corresponding to @@ -52,23 +51,5 @@ func ModeFromString(mode string) (Mode, error) { case "nearest": return NearestMode, nil } - return Mode(0), fmt.Errorf("unknown read preference %v", mode) -} - -// String returns the string representation of mode. -func (mode Mode) String() string { - switch mode { - case PrimaryMode: - return "primary" - case PrimaryPreferredMode: - return "primaryPreferred" - case SecondaryMode: - return "secondary" - case SecondaryPreferredMode: - return "secondaryPreferred" - case NearestMode: - return "nearest" - default: - return "unknown" - } + return "", fmt.Errorf("unknown read preference %v", mode) } diff --git a/mongo/readpref/mode_test.go b/mongo/readpref/mode_test.go deleted file mode 100644 index e92d4a35fb..0000000000 --- a/mongo/readpref/mode_test.go +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (C) MongoDB, Inc. 2020-present. - -// Licensed under the Apache License, Version 2.0 (the "License"); you may -// not use this file except in compliance with the License. You may obtain -// a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - -package readpref - -import ( - "testing" - - "go.mongodb.org/mongo-driver/v2/internal/assert" -) - -func TestMode_String(t *testing.T) { - t.Parallel() - - testCases := []struct { - name string - mode Mode - }{ - {"primary", PrimaryMode}, - {"primaryPreferred", PrimaryPreferredMode}, - {"secondary", SecondaryMode}, - {"secondaryPreferred", SecondaryPreferredMode}, - {"nearest", NearestMode}, - {"unknown", Mode(42)}, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.name, tc.mode.String(), "expected %q, got %q", tc.name, tc.mode.String()) - }) - } -} diff --git a/mongo/readpref/readpref.go b/mongo/readpref/readpref.go index 194c0a35d4..7abb9df44c 100644 --- a/mongo/readpref/readpref.go +++ b/mongo/readpref/readpref.go @@ -163,7 +163,7 @@ func (r *ReadPref) HedgeEnabled() *bool { // String returns a human-readable description of the read preference. func (r *ReadPref) String() string { var b bytes.Buffer - b.WriteString(r.Mode.String()) + b.WriteString(string(r.Mode)) delim := "(" if r.MaxStaleness() != nil { fmt.Fprintf(&b, "%smaxStaleness=%v", delim, *r.MaxStaleness()) diff --git a/mongo/readpref/readpref_test.go b/mongo/readpref/readpref_test.go index 2d7bf41123..27bdbebe42 100644 --- a/mongo/readpref/readpref_test.go +++ b/mongo/readpref/readpref_test.go @@ -8,13 +8,22 @@ package readpref import ( "testing" + "time" "go.mongodb.org/mongo-driver/v2/internal/assert" + "go.mongodb.org/mongo-driver/v2/internal/ptrutil" ) func TestNew(t *testing.T) { t.Parallel() + tagSets := []TagSet{ + { + {Name: "a", Value: "1"}, + {Name: "b", Value: "2"}, + }, + } + tests := []struct { name string mode Mode @@ -23,7 +32,7 @@ func TestNew(t *testing.T) { wantErr error }{ { - name: "primary no options", + name: "primary", mode: PrimaryMode, opts: nil, want: &ReadPref{Mode: PrimaryMode}, @@ -57,6 +66,53 @@ func TestNew(t *testing.T) { want: &ReadPref{Mode: PrimaryPreferredMode}, wantErr: nil, }, + { + name: "primaryPreferred with options", + mode: PrimaryPreferredMode, + opts: []*Builder{Options().SetMaxStaleness(1).SetTagSets(tagSets)}, + want: &ReadPref{ + Mode: PrimaryPreferredMode, + maxStaleness: ptrutil.Ptr[time.Duration](1), + tagSets: tagSets, + }, + wantErr: nil, + }, + { + name: "secondary", + mode: SecondaryMode, + opts: nil, + want: &ReadPref{Mode: SecondaryMode}, + wantErr: nil, + }, + { + name: "secondary with options", + mode: SecondaryMode, + opts: []*Builder{Options().SetMaxStaleness(1).SetTagSets(tagSets)}, + want: &ReadPref{ + Mode: SecondaryMode, + maxStaleness: ptrutil.Ptr[time.Duration](1), + tagSets: tagSets, + }, + wantErr: nil, + }, + { + name: "nearest", + mode: NearestMode, + opts: nil, + want: &ReadPref{Mode: NearestMode}, + wantErr: nil, + }, + { + name: "nearest with options", + mode: NearestMode, + opts: []*Builder{Options().SetMaxStaleness(1).SetTagSets(tagSets)}, + want: &ReadPref{ + Mode: NearestMode, + maxStaleness: ptrutil.Ptr[time.Duration](1), + tagSets: tagSets, + }, + wantErr: nil, + }, } for _, test := range tests { @@ -83,127 +139,31 @@ func TestNew(t *testing.T) { } } -// -//func TestPrimaryPreferred(t *testing.T) { -// subject := PrimaryPreferred() -// -// require.Equal(t, PrimaryPreferredMode, subject.Mode()) -// _, set := subject.MaxStaleness() -// require.False(t, set) -// require.Len(t, subject.TagSets(), 0) -//} -// -//func TestPrimaryPreferred_with_options(t *testing.T) { -// subject := PrimaryPreferred( -// WithMaxStaleness(time.Duration(10)), -// WithTags("a", "1", "b", "2"), -// ) -// -// require.Equal(t, PrimaryPreferredMode, subject.Mode()) -// ms, set := subject.MaxStaleness() -// require.True(t, set) -// require.Equal(t, time.Duration(10), ms) -// require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) -//} -// -//func TestSecondaryPreferred(t *testing.T) { -// subject := SecondaryPreferred() -// -// require.Equal(t, SecondaryPreferredMode, subject.Mode()) -// _, set := subject.MaxStaleness() -// require.False(t, set) -// require.Len(t, subject.TagSets(), 0) -//} -// -//func TestSecondaryPreferred_with_options(t *testing.T) { -// subject := SecondaryPreferred( -// WithMaxStaleness(time.Duration(10)), -// WithTags("a", "1", "b", "2"), -// ) -// -// require.Equal(t, SecondaryPreferredMode, subject.Mode()) -// ms, set := subject.MaxStaleness() -// require.True(t, set) -// require.Equal(t, time.Duration(10), ms) -// require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) -//} -// -//func TestSecondary(t *testing.T) { -// subject := Secondary() -// -// require.Equal(t, SecondaryMode, subject.Mode()) -// _, set := subject.MaxStaleness() -// require.False(t, set) -// require.Len(t, subject.TagSets(), 0) -//} -// -//func TestSecondary_with_options(t *testing.T) { -// subject := Secondary( -// WithMaxStaleness(time.Duration(10)), -// WithTags("a", "1", "b", "2"), -// ) -// -// require.Equal(t, SecondaryMode, subject.Mode()) -// ms, set := subject.MaxStaleness() -// require.True(t, set) -// require.Equal(t, time.Duration(10), ms) -// require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) -//} -// -//func TestNearest(t *testing.T) { -// subject := Nearest() -// -// require.Equal(t, NearestMode, subject.Mode()) -// _, set := subject.MaxStaleness() -// require.False(t, set) -// require.Len(t, subject.TagSets(), 0) -//} -// -//func TestNearest_with_options(t *testing.T) { -// subject := Nearest( -// WithMaxStaleness(time.Duration(10)), -// WithTags("a", "1", "b", "2"), -// ) -// -// require.Equal(t, NearestMode, subject.Mode()) -// ms, set := subject.MaxStaleness() -// require.True(t, set) -// require.Equal(t, time.Duration(10), ms) -// require.Equal(t, []tag.Set{{tag.Tag{Name: "a", Value: "1"}, tag.Tag{Name: "b", Value: "2"}}}, subject.TagSets()) -//} -// -//func TestHedge(t *testing.T) { -// t.Run("hedge specified with primary mode errors", func(t *testing.T) { -// _, err := New(PrimaryMode, WithHedgeEnabled(true)) -// assert.Equal(t, errInvalidReadPreference, err, "expected error %v, got %v", errInvalidReadPreference, err) -// }) -// t.Run("valid hedge document and mode succeeds", func(t *testing.T) { -// rp, err := New(SecondaryMode, WithHedgeEnabled(true)) -// assert.Nil(t, err, "expected no error, got %v", err) -// enabled := rp.HedgeEnabled() -// assert.NotNil(t, enabled, "expected HedgeEnabled to return a non-nil value, got nil") -// assert.True(t, *enabled, "expected HedgeEnabled to return true, got false") -// }) -//} -// -//func TestReadPref_String(t *testing.T) { -// t.Run("ReadPref.String() with all options", func(t *testing.T) { -// readPref := Nearest( -// WithMaxStaleness(120*time.Second), -// WithTagSets(tag.Set{{"a", "1"}, {"b", "2"}}, tag.Set{{"q", "5"}, {"r", "6"}}), -// WithHedgeEnabled(true), -// ) -// expected := "nearest(maxStaleness=2m0s tagSet=a=1,b=2 tagSet=q=5,r=6 hedgeEnabled=true)" -// assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) -// }) -// t.Run("ReadPref.String() with one option", func(t *testing.T) { -// readPref := Secondary(WithTags("a", "1", "b", "2")) -// expected := "secondary(tagSet=a=1,b=2)" -// assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) -// }) -// t.Run("ReadPref.String() with no options", func(t *testing.T) { -// readPref := Primary() -// expected := "primary" -// assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) -// }) -//} +func TestReadPref_String(t *testing.T) { + t.Run("ReadPref.String() with all options", func(t *testing.T) { + opts := Options().SetMaxStaleness(120 * time.Second).SetHedgeEnabled(true).SetTagSets([]TagSet{ + {{"a", "1"}, {"b", "2"}}, + {{"q", "5"}, {"r", "6"}}, + }) + + readPref, err := New(NearestMode, opts) + assert.NoError(t, err) + + expected := "nearest(maxStaleness=2m0s tagSet=a=1,b=2 tagSet=q=5,r=6 hedgeEnabled=true)" + assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) + }) + t.Run("ReadPref.String() with one option", func(t *testing.T) { + opts := Options().SetTagSets([]TagSet{{{"a", "1"}, {"b", "2"}}}) + + readPref, err := New(SecondaryMode, opts) + assert.NoError(t, err) + + expected := "secondary(tagSet=a=1,b=2)" + assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) + }) + t.Run("ReadPref.String() with no options", func(t *testing.T) { + readPref := Primary() + expected := "primary" + assert.Equal(t, expected, readPref.String(), "expected %q, got %q", expected, readPref.String()) + }) +}