GODRIVER-1099 Change RetryType to Type

Change-Id: Ib04da47943a4113b20190c021c7cd921b58519be

Author: iwysiu

x/mongo/driver/connstring/connstring_spec_test.go

@@ -96,20 +96,20 @@ func runTestsInFile(t *testing.T, dirname string, filename string, warningsError
 }
 
 var skipTest = map[string]struct{}{
-	"tlsAllowInvalidHostnames and tlsInsecure both present (and false) raises an error":    struct{}{},
-	"tlsAllowInvalidHostnames and tlsInsecure both present (and true) raises an error":     struct{}{},
-	"tlsInsecure and tlsAllowInvalidHostnames both present (and false) raises an error":    struct{}{},
-	"tlsInsecure and tlsAllowInvalidHostnames both present (and true) raises an error":     struct{}{},
-	"tlsAllowInvalidCertificates and tlsInsecure both present (and false) raises an error": struct{}{},
-	"tlsAllowInvalidCertificates and tlsInsecure both present (and true) raises an error":  struct{}{},
-	"tlsInsecure and tlsAllowInvalidCertificates both present (and false) raises an error": struct{}{},
-	"tlsInsecure and tlsAllowInvalidCertificates both present (and true) raises an error":  struct{}{},
-	"Invalid tlsAllowInvalidHostnames causes a warning":                                    struct{}{},
-	"tlsAllowInvalidHostnames is parsed correctly":                                         struct{}{},
-	"Invalid tlsAllowInvalidCertificates causes a warning":                                 struct{}{},
-	"tlsAllowInvalidCertificates is parsed correctly":                                      struct{}{},
-	"Invalid serverSelectionTryOnce causes a warning":                                      struct{}{},
-	"Valid options specific to single-threaded drivers are parsed correctly":               struct{}{},
+	"tlsAllowInvalidHostnames and tlsInsecure both present (and false) raises an error":    {},
+	"tlsAllowInvalidHostnames and tlsInsecure both present (and true) raises an error":     {},
+	"tlsInsecure and tlsAllowInvalidHostnames both present (and false) raises an error":    {},
+	"tlsInsecure and tlsAllowInvalidHostnames both present (and true) raises an error":     {},
+	"tlsAllowInvalidCertificates and tlsInsecure both present (and false) raises an error": {},
+	"tlsAllowInvalidCertificates and tlsInsecure both present (and true) raises an error":  {},
+	"tlsInsecure and tlsAllowInvalidCertificates both present (and false) raises an error": {},
+	"tlsInsecure and tlsAllowInvalidCertificates both present (and true) raises an error":  {},
+	"Invalid tlsAllowInvalidHostnames causes a warning":                                    {},
+	"tlsAllowInvalidHostnames is parsed correctly":                                         {},
+	"Invalid tlsAllowInvalidCertificates causes a warning":                                 {},
+	"tlsAllowInvalidCertificates is parsed correctly":                                      {},
+	"Invalid serverSelectionTryOnce causes a warning":                                      {},
+	"Valid options specific to single-threaded drivers are parsed correctly":               {},
 }
 
 func runTest(t *testing.T, filename string, test *testCase, warningsError bool) {

x/mongo/driver/driver.go

@@ -120,20 +120,20 @@ type nopCloserConnection struct{ Connection }
 
 func (ncc nopCloserConnection) Close() error { return nil }
 
-// TODO(GODRUVER-617): We can likely use 1 type for both the RetryType and the RetryMode by using
+// TODO(GODRIVER-617): We can likely use 1 type for both the Type and the RetryMode by using
 // 2 bits for the mode and 1 bit for the type. Although in the practical sense, we might not want to
 // do that since the type of retryability is tied to the operation itself and isn't going change,
 // e.g. and insert operation will always be a write, however some operations are both reads and
 // writes, for instance aggregate is a read but with a $out parameter it's a write.
 
-// RetryType specifies whether a retry is a read, write, or disabled.
-type RetryType uint
+// Type specifies whether an operation is a read, write, or unknown.
+type Type uint
 
-// THese are the availables types of retry.
+// THese are the availables types of Type.
 const (
-	_ RetryType = iota
-	RetryWrite
-	RetryRead
+	_ Type = iota
+	Write
+	Read
 )
 
 // RetryMode specifies the way that retries are handled for retryable operations.

x/mongo/driver/drivergen/drivergen.go

@@ -154,13 +154,17 @@ func (op Operation) CommandMethod() (string, error) {
 			return "", fmt.Errorf("unknown request field type %s", field.Type)
 		}
 		var rf struct {
-			ShortName     string
-			Name          string
-			ParameterName string
+			ShortName              string
+			Name                   string
+			ParameterName          string
+			MinWireVersion         int
+			MinWireVersionRequired int
 		}
 		rf.ShortName = op.ShortName()
 		rf.Name = op.Command.Parameter
 		rf.ParameterName = op.Command.Name
+		rf.MinWireVersion = field.MinWireVersion
+		rf.MinWireVersionRequired = field.MinWireVersionRequired
 		err := tmpl.Execute(&buf, rf)
 		if err != nil {
 			return "", err
@@ -173,7 +177,7 @@ func (op Operation) CommandMethod() (string, error) {
 	sort.Strings(names)
 	for _, name := range names {
 		field := op.Request[name]
-		if name == op.Properties.Batches {
+		if name == op.Properties.Batches || field.Skip {
 			continue
 		}
 		var tmpl *template.Template
@@ -538,6 +542,7 @@ type RequestField struct {
 	Slice                  bool
 	Constructor            bool
 	Variadic               bool
+	Skip                   bool
 	Documentation          string
 	MinWireVersion         int
 	MinWireVersionRequired int

x/mongo/driver/drivergen/templates/operation.tmpl

@@ -179,7 +179,7 @@ func ({{$.ShortName}} *{{$.Name}}) Execute(ctx context.Context) error {
         {{- end -}}
 
         {{- if eq $.Properties.Retryable.Type "writes"}}
-        RetryType: driver.RetryWrite,
+        Type: driver.Write,
         {{- end -}}
 
         {{- range $b := $.Properties.ExecuteBuiltins}}

x/mongo/driver/operation.go

@@ -148,14 +148,13 @@ type Operation struct {
 
 	// RetryMode specifies how to retry. There are three modes that enable retry: RetryOnce,
 	// RetryOncePerCommand, and RetryContext. For more information about what these modes do, please
-	// refer to their definitions. Both RetryMode and RetryType must be set for retryability to be
-	// enabled.
+	// refer to their definitions. Both RetryMode and Type must be set for retryability to be enabled.
 	RetryMode *RetryMode
 
-	// RetryType specifies the kinds of operations that can be retried. There is only one mode that
-	// enables retry: RetryWrites. For more information about what this mode does, please refer to
-	// it's definition. Both RetryType and RetryMode must be set for retryability to be enabled.
-	RetryType RetryType
+	// Type specifies the kind of operation this is. There is only one mode that enables retry: Write.
+	// For more information about what this mode does, please refer to it's definition. Both Type and
+	// RetryMode must be set for retryability to be enabled.
+	Type Type
 
 	// Batches contains the documents that are split when executing a write command that potentially
 	// has more documents than can fit in a single command. This should only be specified for
@@ -264,7 +263,7 @@ func (op Operation) Execute(ctx context.Context, scratch []byte) error {
 	var retries int
 	// TODO(GODRIVER-617): Add support for retryable reads.
 	retryable := op.retryable(desc.Server)
-	if retryable == RetryWrite && op.Client != nil && op.RetryMode != nil {
+	if retryable == Write && op.Client != nil && op.RetryMode != nil {
 		op.Client.RetryWrite = false
 		if *op.RetryMode > RetryNone {
 			op.Client.RetryWrite = true
@@ -350,7 +349,7 @@ func (op Operation) Execute(ctx context.Context, scratch []byte) error {
 
 		switch tt := err.(type) {
 		case WriteCommandError:
-			if retryable == RetryWrite && tt.Retryable() && retries != 0 {
+			if retryable == Write && tt.Retryable() && retries != 0 {
 				retries--
 				original, err = err, nil
 				conn.Close() // Avoid leaking the connection.
@@ -359,7 +358,7 @@ func (op Operation) Execute(ctx context.Context, scratch []byte) error {
 					return original
 				}
 				conn, err = srvr.Connection(ctx)
-				if err != nil || conn == nil || op.retryable(conn.Description()) != RetryWrite {
+				if err != nil || conn == nil || op.retryable(conn.Description()) != Write {
 					if conn != nil {
 						conn.Close()
 					}
@@ -396,7 +395,7 @@ func (op Operation) Execute(ctx context.Context, scratch []byte) error {
 			if tt.HasErrorLabel(TransientTransactionError) || tt.HasErrorLabel(UnknownTransactionCommitResult) {
 				op.Client.ClearPinnedServer()
 			}
-			if retryable == RetryWrite && tt.Retryable() && retries != 0 {
+			if retryable == Write && tt.Retryable() && retries != 0 {
 				retries--
 				original, err = err, nil
 				conn.Close() // Avoid leaking the connection.
@@ -405,7 +404,7 @@ func (op Operation) Execute(ctx context.Context, scratch []byte) error {
 					return original
 				}
 				conn, err = srvr.Connection(ctx)
-				if err != nil || conn == nil || op.retryable(conn.Description()) != RetryWrite {
+				if err != nil || conn == nil || op.retryable(conn.Description()) != Write {
 					if conn != nil {
 						conn.Close()
 					}
@@ -436,7 +435,7 @@ func (op Operation) Execute(ctx context.Context, scratch []byte) error {
 		}
 
 		if batching && len(op.Batches.Documents) > 0 {
-			if retryable == RetryWrite && op.Client != nil && op.RetryMode != nil {
+			if retryable == Write && op.Client != nil && op.RetryMode != nil {
 				if *op.RetryMode > RetryNone {
 					op.Client.IncrementTxnNumber()
 				}
@@ -457,20 +456,20 @@ func (op Operation) Execute(ctx context.Context, scratch []byte) error {
 
 // Retryable writes are supported if the server supports sessions, the operation is not
 // within a transaction, and the write is acknowledged
-func (op Operation) retryable(desc description.Server) RetryType {
-	switch op.RetryType {
-	case RetryWrite:
+func (op Operation) retryable(desc description.Server) Type {
+	switch op.Type {
+	case Write:
 		if op.Client != nil && (op.Client.Committing || op.Client.Aborting) {
-			return RetryWrite
+			return Write
 		}
 		if op.Deployment.SupportsRetry() &&
 			description.SessionsSupported(desc.WireVersion) &&
 			op.Client != nil && !(op.Client.TransactionInProgress() || op.Client.TransactionStarting()) &&
 			writeconcern.AckWrite(op.WriteConcern) {
-			return RetryWrite
+			return Write
 		}
 	}
-	return RetryType(0)
+	return Type(0)
 }
 
 // roundTrip writes a wiremessage to the connection and then reads a wiremessage. The wm parameter
@@ -811,7 +810,7 @@ func (op Operation) addSession(dst []byte, desc description.SelectedServer) ([]b
 	dst = bsoncore.AppendDocumentElement(dst, "lsid", lsid)
 
 	var addedTxnNumber bool
-	if op.RetryType == RetryWrite && client != nil && client.RetryWrite {
+	if op.Type == Write && client != nil && client.RetryWrite {
 		addedTxnNumber = true
 		dst = bsoncore.AppendInt64Element(dst, "txnNumber", op.Client.TxnNumber)
 	}
@@ -896,11 +895,6 @@ func (op Operation) updateOperationTime(response bsoncore.Document) {
 }
 
 func (op Operation) getReadPrefBasedOnTransaction() (*readpref.ReadPref, error) {
-	// don't validate read preference for write commands
-	if op.RetryType == RetryWrite {
-		return op.ReadPreference, nil
-	}
-
 	if op.Client != nil && op.Client.TransactionRunning() {
 		// Transaction's read preference always takes priority
 		rp := op.Client.CurrentRp
@@ -915,7 +909,7 @@ func (op Operation) getReadPrefBasedOnTransaction() (*readpref.ReadPref, error)
 }
 
 func (op Operation) createReadPref(serverKind description.ServerKind, topologyKind description.TopologyKind, isOpQuery bool) (bsoncore.Document, error) {
-	if serverKind == description.Standalone || (isOpQuery && serverKind != description.Mongos) {
+	if serverKind == description.Standalone || (isOpQuery && serverKind != description.Mongos) || op.Type == Write {
 		// Don't send read preference for non-mongos when using OP_QUERY or for all standalones
 		return nil, nil
 	}