From a10150b0070a7882af25a7946cf8cd438ed0e3ef Mon Sep 17 00:00:00 2001 From: Matt Dale <9760375+matthewdale@users.noreply.github.com> Date: Wed, 14 Jun 2023 14:12:30 -0700 Subject: [PATCH] GODRIVER-2685 Simplify the writeconcern API. (#1232) --- mongo/writeconcern/writeconcern.go | 328 ++++++++++++++---- .../writeconcern/writeconcern_example_test.go | 60 ++++ mongo/writeconcern/writeconcern_test.go | 219 +++++++++++- 3 files changed, 537 insertions(+), 70 deletions(-) create mode 100644 mongo/writeconcern/writeconcern_example_test.go diff --git a/mongo/writeconcern/writeconcern.go b/mongo/writeconcern/writeconcern.go index c9717725c4..8e288d10b7 100644 --- a/mongo/writeconcern/writeconcern.go +++ b/mongo/writeconcern/writeconcern.go @@ -5,10 +5,14 @@ // a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 // Package writeconcern defines write concerns for MongoDB operations. +// +// For more information about MongoDB write concerns, see +// https://www.mongodb.com/docs/manual/reference/write-concern/ package writeconcern // import "go.mongodb.org/mongo-driver/mongo/writeconcern" import ( "errors" + "fmt" "time" "go.mongodb.org/mongo-driver/bson" @@ -16,35 +20,160 @@ import ( "go.mongodb.org/mongo-driver/x/bsonx/bsoncore" ) +const majority = "majority" + // ErrInconsistent indicates that an inconsistent write concern was specified. +// +// Deprecated: ErrInconsistent will be removed in Go Driver 2.0. var ErrInconsistent = errors.New("a write concern cannot have both w=0 and j=true") // ErrEmptyWriteConcern indicates that a write concern has no fields set. +// +// Deprecated: ErrEmptyWriteConcern will be removed in Go Driver 2.0. var ErrEmptyWriteConcern = errors.New("a write concern must have at least one field set") // ErrNegativeW indicates that a negative integer `w` field was specified. +// +// Deprecated: ErrNegativeW will be removed in Go Driver 2.0. var ErrNegativeW = errors.New("write concern `w` field cannot be a negative number") // ErrNegativeWTimeout indicates that a negative WTimeout was specified. +// +// Deprecated: ErrNegativeWTimeout will be removed in Go Driver 2.0. var ErrNegativeWTimeout = errors.New("write concern `wtimeout` field cannot be negative") -// WriteConcern describes the level of acknowledgement requested from MongoDB for write operations -// to a standalone mongod or to replica sets or to sharded clusters. +// A WriteConcern defines a MongoDB read concern, which describes the level of acknowledgment +// requested from MongoDB for write operations to a standalone mongod, to replica sets, or to +// sharded clusters. +// +// For more information about MongoDB write concerns, see +// https://www.mongodb.com/docs/manual/reference/write-concern/ type WriteConcern struct { - w interface{} - j bool + // W requests acknowledgment that the write operation has propagated to a + // specified number of mongod instances or to mongod instances with + // specified tags. It sets the the "w" option in a MongoDB write concern. + // + // W values must be a string or an int. + // + // Common values are: + // - "majority": requests acknowledgment that write operations have been + // durably committed to the calculated majority of the data-bearing + // voting members. + // - 1: requests acknowledgment that write operations have been written + // to 1 node. + // - 0: requests no acknowledgment of write operations + // + // For more information about the "w" option, see + // https://www.mongodb.com/docs/manual/reference/write-concern/#w-option + W interface{} + + // Journal requests acknowledgment from MongoDB that the write operation has + // been written to the on-disk journal. It sets the "j" option in a MongoDB + // write concern. + // + // For more information about the "j" option, see + // https://www.mongodb.com/docs/manual/reference/write-concern/#j-option + Journal *bool + + // WTimeout specifies a time limit for the write concern. It sets the + // "wtimeout" option in a MongoDB write concern. + // + // It is only applicable for "w" values greater than 1. Using a WTimeout and + // setting Timeout on the Client at the same time will result in undefined + // behavior. + // + // For more information about the "wtimeout" option, see + // https://www.mongodb.com/docs/manual/reference/write-concern/#wtimeout + WTimeout time.Duration +} + +// Unacknowledged returns a WriteConcern that requests no acknowledgment of +// write operations. +// +// For more information about write concern "w: 0", see +// https://www.mongodb.com/docs/manual/reference/write-concern/#mongodb-writeconcern-writeconcern.-number- +func Unacknowledged() *WriteConcern { + return &WriteConcern{W: 0} +} + +// W1 returns a WriteConcern that requests acknowledgment that write operations +// have been written to memory on one node (e.g. the standalone mongod or the +// primary in a replica set). +// +// For more information about write concern "w: 1", see +// https://www.mongodb.com/docs/manual/reference/write-concern/#mongodb-writeconcern-writeconcern.-number- +func W1() *WriteConcern { + return &WriteConcern{W: 1} +} + +// Journaled returns a WriteConcern that requests acknowledgment that write +// operations have been written to the on-disk journal on MongoDB. +// +// The database's default value for "w" determines how many nodes must write to +// their on-disk journal before the write operation is acknowledged. +// +// For more information about write concern "j: true", see +// https://www.mongodb.com/docs/manual/reference/write-concern/#mongodb-writeconcern-ournal +func Journaled() *WriteConcern { + journal := true + return &WriteConcern{Journal: &journal} +} - // NOTE(benjirewis): wTimeout will be deprecated in a future release. The more general Timeout - // option may be used in its place to control the amount of time that a single operation can run - // before returning an error. Using wTimeout and setting Timeout on the client will result in - // undefined behavior. - wTimeout time.Duration +// Majority returns a WriteConcern that requests acknowledgment that write +// operations have been durably committed to the calculated majority of the +// data-bearing voting members. +// +// Write concern "w: majority" typically requires write operations to be written +// to the on-disk journal before they are acknowledged, unless journaling is +// disabled on MongoDB or the "writeConcernMajorityJournalDefault" replica set +// configuration is set to false. +// +// For more information about write concern "w: majority", see +// https://www.mongodb.com/docs/manual/reference/write-concern/#mongodb-writeconcern-writeconcern.-majority- +func Majority() *WriteConcern { + return &WriteConcern{W: majority} +} + +// Custom returns a WriteConcern that requests acknowledgment that write +// operations have propagated to tagged members that satisfy the custom write +// concern defined in "settings.getLastErrorModes". +// +// For more information about custom write concern names, see +// https://www.mongodb.com/docs/manual/reference/write-concern/#mongodb-writeconcern-writeconcern.-custom-write-concern-name- +func Custom(tag string) *WriteConcern { + return &WriteConcern{W: tag} } // Option is an option to provide when creating a WriteConcern. +// +// Deprecated: Use the WriteConcern convenience functions or define a struct literal instead. +// For example: +// +// writeconcern.Majority() +// +// or +// +// journal := true +// &writeconcern.WriteConcern{ +// W: 2, +// Journal: &journal, +// } type Option func(concern *WriteConcern) // New constructs a new WriteConcern. +// +// Deprecated: Use the WriteConcern convenience functions or define a struct literal instead. +// For example: +// +// writeconcern.Majority() +// +// or +// +// journal := true +// &writeconcern.WriteConcern{ +// W: 2, +// Journal: &journal, +// } func New(options ...Option) *WriteConcern { concern := &WriteConcern{} @@ -57,85 +186,147 @@ func New(options ...Option) *WriteConcern { // W requests acknowledgement that write operations propagate to the specified number of mongod // instances. +// +// Deprecated: Use the Unacknowledged or W1 functions or define a struct literal instead. +// For example: +// +// writeconcern.Unacknowledged() +// +// or +// +// journal := true +// &writeconcern.WriteConcern{ +// W: 2, +// Journal: &journal, +// } func W(w int) Option { return func(concern *WriteConcern) { - concern.w = w + concern.W = w } } // WMajority requests acknowledgement that write operations propagate to the majority of mongod // instances. +// +// Deprecated: Use [Majority] instead. func WMajority() Option { return func(concern *WriteConcern) { - concern.w = "majority" + concern.W = majority } } // WTagSet requests acknowledgement that write operations propagate to the specified mongod // instance. +// +// Deprecated: Use [Custom] instead. func WTagSet(tag string) Option { return func(concern *WriteConcern) { - concern.w = tag + concern.W = tag } } // J requests acknowledgement from MongoDB that write operations are written to // the journal. +// +// Deprecated: Use the Journaled function or define a struct literal instead. +// For example: +// +// writeconcern.Journaled() +// +// or +// +// journal := true +// &writeconcern.WriteConcern{ +// W: 2, +// Journal: &journal, +// } func J(j bool) Option { return func(concern *WriteConcern) { - concern.j = j + // To maintain backward compatible behavior (now that the J field is a + // bool pointer), only set a value for J if the input is true. If the + // input is false, do not set a value, which omits "j" from the + // marshaled write concern. + if j { + concern.Journal = &j + } } } -// WTimeout specifies specifies a time limit for the write concern. +// WTimeout specifies a time limit for the write concern. +// +// It is only applicable for "w" values greater than 1. Using a WTimeout and setting Timeout on the +// Client at the same time will result in undefined behavior. +// +// Deprecated: Use the WriteConcern convenience functions or define a struct literal instead. +// For example: +// +// wc := writeconcern.W1() +// wc.WTimeout = 30 * time.Second // -// NOTE(benjirewis): wTimeout will be deprecated in a future release. The more general Timeout -// option may be used in its place to control the amount of time that a single operation can run -// before returning an error. Using wTimeout and setting Timeout on the client will result in -// undefined behavior. +// or +// +// journal := true +// &writeconcern.WriteConcern{ +// W: "majority", +// WTimeout: 30 * time.Second, +// } func WTimeout(d time.Duration) Option { return func(concern *WriteConcern) { - concern.wTimeout = d + concern.WTimeout = d } } // MarshalBSONValue implements the bson.ValueMarshaler interface. +// +// Deprecated: Marshaling a WriteConcern to BSON will not be supported in Go +// Driver 2.0. func (wc *WriteConcern) MarshalBSONValue() (bsontype.Type, []byte, error) { - if !wc.IsValid() { - return bsontype.Type(0), nil, ErrInconsistent + if wc == nil { + return 0, nil, ErrEmptyWriteConcern } var elems []byte - - if wc.w != nil { - switch t := wc.w.(type) { + if wc.W != nil { + // Only support string or int values for W. That aligns with the + // documentation and the behavior of other functions, like Acknowledged. + switch w := wc.W.(type) { case int: - if t < 0 { - return bsontype.Type(0), nil, ErrNegativeW + if w < 0 { + return 0, nil, ErrNegativeW } - elems = bsoncore.AppendInt32Element(elems, "w", int32(t)) + // If Journal=true and W=0, return an error because that write + // concern is ambiguous. + if wc.Journal != nil && *wc.Journal && w == 0 { + return 0, nil, ErrInconsistent + } + + elems = bsoncore.AppendInt32Element(elems, "w", int32(w)) case string: - elems = bsoncore.AppendStringElement(elems, "w", t) + elems = bsoncore.AppendStringElement(elems, "w", w) + default: + return 0, + nil, + fmt.Errorf("WriteConcern.W must be a string or int, but is a %T", wc.W) } } - if wc.j { - elems = bsoncore.AppendBooleanElement(elems, "j", wc.j) + if wc.Journal != nil { + elems = bsoncore.AppendBooleanElement(elems, "j", *wc.Journal) } - if wc.wTimeout < 0 { - return bsontype.Type(0), nil, ErrNegativeWTimeout + if wc.WTimeout < 0 { + return 0, nil, ErrNegativeWTimeout } - if wc.wTimeout != 0 { - elems = bsoncore.AppendInt64Element(elems, "wtimeout", int64(wc.wTimeout/time.Millisecond)) + if wc.WTimeout != 0 { + elems = bsoncore.AppendInt64Element(elems, "wtimeout", int64(wc.WTimeout/time.Millisecond)) } if len(elems) == 0 { - return bsontype.Type(0), nil, ErrEmptyWriteConcern + return 0, nil, ErrEmptyWriteConcern } - return bsontype.EmbeddedDocument, bsoncore.BuildDocument(nil, elems), nil + return bson.TypeEmbeddedDocument, bsoncore.BuildDocument(nil, elems), nil } // AcknowledgedValue returns true if a BSON RawValue for a write concern represents an acknowledged write concern. @@ -163,52 +354,69 @@ func AcknowledgedValue(rawv bson.RawValue) bool { // Acknowledged indicates whether or not a write with the given write concern will be acknowledged. func (wc *WriteConcern) Acknowledged() bool { - if wc == nil || wc.j { - return true - } - - switch v := wc.w.(type) { - case int: - if v == 0 { - return false - } - } - - return true + // Only {w: 0} or {w: 0, j: false} are an unacknowledged write concerns. All other values are + // acknowledged. + return wc == nil || wc.W != 0 || (wc.Journal != nil && *wc.Journal) } -// IsValid checks whether the write concern is invalid. +// IsValid returns true if the WriteConcern is valid. func (wc *WriteConcern) IsValid() bool { - if !wc.j { + if wc == nil { return true } - switch v := wc.w.(type) { + switch w := wc.W.(type) { case int: - if v == 0 { - return false - } + // A write concern with {w: int} must have a non-negative value and + // cannot have the combination {w: 0, j: true}. + return w >= 0 && (w > 0 || wc.Journal == nil || !*wc.Journal) + case string, nil: + // A write concern with {w: string} or no w specified is always valid. + return true + default: + // A write concern with an unsupported w type is not valid. + return false } - - return true } // GetW returns the write concern w level. +// +// Deprecated: Use the WriteConcern.W field instead. func (wc *WriteConcern) GetW() interface{} { - return wc.w + return wc.W } // GetJ returns the write concern journaling level. +// +// Deprecated: Use the WriteConcern.Journal field instead. func (wc *WriteConcern) GetJ() bool { - return wc.j + // Treat a nil Journal as false. That maintains backward compatibility with the existing + // behavior of GetJ where unset is false. If users want the real value of Journal, they can + // access the Journal field. + return wc.Journal != nil && *wc.Journal } // GetWTimeout returns the write concern timeout. +// +// Deprecated: Use the WriteConcern.WTimeout field instead. func (wc *WriteConcern) GetWTimeout() time.Duration { - return wc.wTimeout + return wc.WTimeout } // WithOptions returns a copy of this WriteConcern with the options set. +// +// Deprecated: Use the WriteConcern convenience functions or define a struct literal instead. +// For example: +// +// writeconcern.Majority() +// +// or +// +// journal := true +// &writeconcern.WriteConcern{ +// W: 2, +// Journal: &journal, +// } func (wc *WriteConcern) WithOptions(options ...Option) *WriteConcern { if wc == nil { return New(options...) @@ -224,6 +432,8 @@ func (wc *WriteConcern) WithOptions(options ...Option) *WriteConcern { } // AckWrite returns true if a write concern represents an acknowledged write +// +// Deprecated: Use [WriteConcern.Acknowledged] instead. func AckWrite(wc *WriteConcern) bool { return wc == nil || wc.Acknowledged() } diff --git a/mongo/writeconcern/writeconcern_example_test.go b/mongo/writeconcern/writeconcern_example_test.go new file mode 100644 index 0000000000..dda4b15e9c --- /dev/null +++ b/mongo/writeconcern/writeconcern_example_test.go @@ -0,0 +1,60 @@ +// 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 writeconcern_test + +import ( + "context" + + "go.mongodb.org/mongo-driver/mongo" + "go.mongodb.org/mongo-driver/mongo/options" + "go.mongodb.org/mongo-driver/mongo/writeconcern" +) + +// Configure a Client with write concern "majority" that requests +// acknowledgement that a majority of the nodes have committed write operations. +func Example_majority() { + wc := writeconcern.Majority() + + opts := options.Client(). + ApplyURI("mongodb://localhost:27017"). + SetWriteConcern(wc) + + _, err := mongo.Connect(context.Background(), opts) + if err != nil { + panic(err) + } +} + +// Configure a Client with a write concern that requests acknowledgement that +// exactly 2 nodes have committed and journaled write operations. +func Example_w2Journaled() { + wc := &writeconcern.WriteConcern{ + W: 2, + Journal: boolPtr(true), + } + + opts := options.Client(). + ApplyURI("mongodb://localhost:27017"). + SetWriteConcern(wc) + + _, err := mongo.Connect(context.Background(), opts) + if err != nil { + panic(err) + } +} + +// boolPtr is a helper function to convert a bool constant into a bool pointer. +// +// If you're using a version of Go that supports generics, you can define a +// generic version of this function that works with any type. For example: +// +// func ptr[T any](v T) *T { +// return &v +// } +func boolPtr(b bool) *bool { + return &b +} diff --git a/mongo/writeconcern/writeconcern_test.go b/mongo/writeconcern/writeconcern_test.go index a323f12e85..459e0eabe3 100644 --- a/mongo/writeconcern/writeconcern_test.go +++ b/mongo/writeconcern/writeconcern_test.go @@ -7,38 +7,235 @@ package writeconcern_test import ( + "errors" "testing" "time" + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/bson/bsontype" + "go.mongodb.org/mongo-driver/internal/assert" "go.mongodb.org/mongo-driver/internal/require" "go.mongodb.org/mongo-driver/mongo/writeconcern" ) func TestWriteConcernWithOptions(t *testing.T) { + t.Parallel() + t.Run("on nil WriteConcern", func(t *testing.T) { + t.Parallel() + var wc *writeconcern.WriteConcern wc = wc.WithOptions(writeconcern.WMajority()) - require.Equal(t, wc.GetW().(string), "majority") + assert.Equal(t, "majority", wc.GetW().(string)) + assert.False(t, wc.GetJ()) }) t.Run("on existing WriteConcern", func(t *testing.T) { + t.Parallel() + wc := writeconcern.New(writeconcern.W(1), writeconcern.J(true)) - require.Equal(t, wc.GetW().(int), 1) - require.Equal(t, wc.GetJ(), true) + assert.Equal(t, 1, wc.GetW().(int)) + assert.True(t, wc.GetJ()) wc = wc.WithOptions(writeconcern.WMajority()) - require.Equal(t, wc.GetW().(string), "majority") - require.Equal(t, wc.GetJ(), true) + assert.Equal(t, "majority", wc.GetW().(string)) + assert.True(t, wc.GetJ()) }) t.Run("with multiple options", func(t *testing.T) { + t.Parallel() + wc := writeconcern.New(writeconcern.W(1), writeconcern.J(true)) - require.Equal(t, wc.GetW().(int), 1) - require.Equal(t, wc.GetJ(), true) - require.Equal(t, wc.GetWTimeout(), time.Duration(0)) + assert.Equal(t, 1, wc.GetW().(int)) + assert.True(t, wc.GetJ()) + assert.Equal(t, time.Duration(0), wc.GetWTimeout()) wc = wc.WithOptions(writeconcern.WMajority(), writeconcern.WTimeout(time.Second)) - require.Equal(t, wc.GetW().(string), "majority") - require.Equal(t, wc.GetJ(), true) - require.Equal(t, wc.GetWTimeout(), time.Second) + assert.Equal(t, "majority", wc.GetW().(string)) + assert.True(t, wc.GetJ()) + assert.Equal(t, time.Second, wc.GetWTimeout()) }) } + +func TestWriteConcern_MarshalBSONValue(t *testing.T) { + t.Parallel() + + boolPtr := func(b bool) *bool { return &b } + + testCases := []struct { + name string + wc *writeconcern.WriteConcern + wantType bsontype.Type + wantValue bson.D + wantError error + }{ + { + name: "all fields", + wc: &writeconcern.WriteConcern{ + W: "majority", + Journal: boolPtr(false), + WTimeout: 1 * time.Minute, + }, + wantType: bson.TypeEmbeddedDocument, + wantValue: bson.D{ + {Key: "w", Value: "majority"}, + {Key: "j", Value: false}, + {Key: "wtimeout", Value: int64(60_000)}, + }, + }, + { + name: "string W", + wc: &writeconcern.WriteConcern{W: "majority"}, + wantType: bson.TypeEmbeddedDocument, + wantValue: bson.D{{Key: "w", Value: "majority"}}, + }, + { + name: "int W", + wc: &writeconcern.WriteConcern{W: 1}, + wantType: bson.TypeEmbeddedDocument, + wantValue: bson.D{{Key: "w", Value: int32(1)}}, + }, + { + name: "int32 W", + wc: &writeconcern.WriteConcern{W: int32(1)}, + wantError: errors.New("WriteConcern.W must be a string or int, but is a int32"), + }, + { + name: "bool W", + wc: &writeconcern.WriteConcern{W: false}, + wantError: errors.New("WriteConcern.W must be a string or int, but is a bool"), + }, + { + name: "W=0 and J=true", + wc: &writeconcern.WriteConcern{W: 0, Journal: boolPtr(true)}, + wantError: writeconcern.ErrInconsistent, + }, + { + name: "negative W", + wc: &writeconcern.WriteConcern{W: -1}, + wantError: writeconcern.ErrNegativeW, + }, + { + name: "negative WTimeout", + wc: &writeconcern.WriteConcern{W: 1, WTimeout: -1}, + wantError: writeconcern.ErrNegativeWTimeout, + }, + { + name: "empty", + wc: &writeconcern.WriteConcern{}, + wantError: writeconcern.ErrEmptyWriteConcern, + }, + { + name: "nil", + wc: nil, + wantError: writeconcern.ErrEmptyWriteConcern, + }, + } + + for _, tc := range testCases { + tc := tc // Capture range variable. + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + typ, b, err := tc.wc.MarshalBSONValue() + if tc.wantError != nil { + assert.Equal(t, tc.wantError, err, "expected and actual errors do not match") + return + } + require.NoError(t, err, "bson.MarshalValue error") + + assert.Equal(t, tc.wantType, typ, "expected and actual BSON types do not match") + + rv := bson.RawValue{ + Type: typ, + Value: b, + } + var gotValue bson.D + err = rv.Unmarshal(&gotValue) + require.NoError(t, err, "error unmarshaling RawValue") + assert.Equal(t, tc.wantValue, gotValue, "expected and actual BSON values do not match") + }) + } +} + +func TestWriteConcern(t *testing.T) { + boolPtr := func(b bool) *bool { return &b } + + testCases := []struct { + name string + wc *writeconcern.WriteConcern + wantAcknowledged bool + wantIsValid bool + }{ + { + name: "Unacknowledged", + wc: writeconcern.Unacknowledged(), + wantAcknowledged: false, + wantIsValid: true, + }, + { + name: "W1", + wc: writeconcern.W1(), + wantAcknowledged: true, + wantIsValid: true, + }, + { + name: "Journaled", + wc: writeconcern.Journaled(), + wantAcknowledged: true, + wantIsValid: true, + }, + { + name: "Majority", + wc: writeconcern.Majority(), + wantAcknowledged: true, + wantIsValid: true, + }, + { + name: "{w: 0, j: true}", + wc: &writeconcern.WriteConcern{ + W: 0, + Journal: boolPtr(true), + }, + wantAcknowledged: true, + wantIsValid: false, + }, + { + name: "{w: custom}", + wc: &writeconcern.WriteConcern{W: "custom"}, + wantAcknowledged: true, + wantIsValid: true, + }, + { + name: "nil", + wc: nil, + wantAcknowledged: true, + wantIsValid: true, + }, + { + name: "invalid type", + wc: &writeconcern.WriteConcern{ + W: struct{ Field string }{}, + }, + wantAcknowledged: true, + wantIsValid: false, + }, + } + + for _, tc := range testCases { + tc := tc // Capture range variable. + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, + tc.wantAcknowledged, + tc.wc.Acknowledged(), + "expected and actual Acknowledged value are different") + assert.Equal(t, + tc.wantIsValid, + tc.wc.IsValid(), + "expected and actual IsValid value are different") + }) + } +}