Merge pull request #1390 from michael-berlin/vitessdriver

vitessdriver: Set default values in OpenWithConfiguration() + fix db.Ping() comment.

go/cmd/vtclient/vtclient.go

@@ -15,6 +15,7 @@ import (
  log "github.com/golang/glog"
  "github.com/youtube/vitess/go/exit"
  "github.com/youtube/vitess/go/vt/logutil"
+ "github.com/youtube/vitess/go/vt/vtgate/vtgateconn"
 
  "github.com/youtube/vitess/go/vt/vitessdriver"
 )
@@ -105,6 +106,7 @@ func main() {
  }
 
  c := vitessdriver.Configuration{
+ Protocol: *vtgateconn.VtgateProtocol,
  Address: *server,
  Keyspace: *keyspace,
  Shard: *shard,

go/vt/vitessdriver/driver.go

@@ -80,11 +80,11 @@ func OpenShardForStreaming(address, keyspace, shard, tabletType string, timeout
 // It allows to pass in a Configuration struct to control all possible
 // settings of the Vitess Go SQL driver.
 func OpenWithConfiguration(c Configuration) (*sql.DB, error) {
- jsonBytes, err := json.Marshal(c)
+ json, err := c.toJSON()
  if err != nil {
  return nil, err
  }
- return sql.Open("vitess", string(jsonBytes))
+ return sql.Open("vitess", json)
 }
 
 type drv struct {
@@ -105,9 +105,6 @@ type drv struct {
 //
 // For a description of the available fields, see the Configuration struct.
 // Note: In the JSON string, timeout has to be specified in nanoseconds.
-//
-// Note that this function will always create a connection to vtgate i.e. there
-// is no need to call DB.Ping() to verify the connection.
 func (d drv) Open(name string) (driver.Conn, error) {
  c := &conn{Configuration: newDefaultConfiguration()}
  err := json.Unmarshal([]byte(name), c)
@@ -175,11 +172,31 @@ type Configuration struct {
 }
 
 func newDefaultConfiguration() Configuration {
- return Configuration{
- Protocol: "grpc",
- TabletType: "master",
- Streaming: false,
+ c := Configuration{}
+ c.setDefaults()
+ return c
+}
+
+// toJSON converts Configuration to the JSON string which is required by the
+// Vitess driver. Default values for empty fields will be set.
+func (c Configuration) toJSON() (string, error) {
+ c.setDefaults()
+ jsonBytes, err := json.Marshal(c)
+ if err != nil {
+ return "", err
+ }
+ return string(jsonBytes), nil
+}
+
+// setDefaults sets the default values for empty fields.
+func (c *Configuration) setDefaults() {
+ if c.Protocol == "" {
+ c.Protocol = "grpc"
+ }
+ if c.TabletType == "" {
+ c.TabletType = "master"
  }
+ // c.Streaming = false is enforced by Go's zero value.
 }
 
 type conn struct {

go/vt/vitessdriver/driver_test.go

@@ -48,54 +48,68 @@ func TestMain(m *testing.M) {
 }
 
 func TestOpen(t *testing.T) {
- connStr := fmt.Sprintf(`{"address": "%s", "tablet_type": "replica", "timeout": %d}`, testAddress, int64(30*time.Second))
- c, err := drv{}.Open(connStr)
- if err != nil {
- t.Fatal(err)
- }
- defer c.Close()
-
- wantc := &conn{
- Configuration: Configuration{
- Protocol: "grpc",
- TabletType: "replica",
- Streaming: false,
- Timeout: 30 * time.Second,
+ var testcases = []struct {
+ desc string
+ connStr string
+ conn *conn
+ }{
+ {
+ desc: "Open() vtgate v3",
+ connStr: fmt.Sprintf(`{"address": "%s", "tablet_type": "replica", "timeout": %d}`, testAddress, int64(30*time.Second)),
+ conn: &conn{
+ Configuration: Configuration{
+ Protocol: "grpc",
+ TabletType: "replica",
+ Streaming: false,
+ Timeout: 30 * time.Second,
+ },
+ tabletTypeProto: topodatapb.TabletType_REPLICA,
+ },
  },
- tabletTypeProto: topodatapb.TabletType_REPLICA,
- }
- newc := *(c.(*conn))
- newc.Address = ""
- newc.vtgateConn = nil
- if !reflect.DeepEqual(&newc, wantc) {
- t.Errorf("conn: %+v, want %+v", &newc, wantc)
- }
-}
-
-func TestOpenShard(t *testing.T) {
- connStr := fmt.Sprintf(`{"address": "%s", "keyspace": "ks1", "shard": "0", "tablet_type": "replica", "timeout": %d}`, testAddress, int64(30*time.Second))
- c, err := drv{}.Open(connStr)
- if err != nil {
- t.Fatal(err)
- }
- defer c.Close()
-
- wantc := &conn{
- Configuration: Configuration{
- Protocol: "grpc",
- Keyspace: "ks1",
- Shard: "0",
- TabletType: "replica",
- Streaming: false,
- Timeout: 30 * time.Second,
+ {
+ desc: "Open() vtgate v3 (defaults omitted)",
+ connStr: fmt.Sprintf(`{"address": "%s", "timeout": %d}`, testAddress, int64(30*time.Second)),
+ conn: &conn{
+ Configuration: Configuration{
+ Protocol: "grpc",
+ TabletType: "master",
+ Streaming: false,
+ Timeout: 30 * time.Second,
+ },
+ tabletTypeProto: topodatapb.TabletType_MASTER,
+ },
+ },
+ {
+ desc: "Open() vtgate v1 (per-shard)",
+ connStr: fmt.Sprintf(`{"address": "%s", "keyspace": "ks1", "shard": "0", "tablet_type": "replica", "timeout": %d}`, testAddress, int64(30*time.Second)),
+ conn: &conn{
+ Configuration: Configuration{
+ Protocol: "grpc",
+ Keyspace: "ks1",
+ Shard: "0",
+ TabletType: "replica",
+ Streaming: false,
+ Timeout: 30 * time.Second,
+ },
+ tabletTypeProto: topodatapb.TabletType_REPLICA,
+ },
  },
- tabletTypeProto: topodatapb.TabletType_REPLICA,
  }
- newc := *(c.(*conn))
- newc.Address = ""
- newc.vtgateConn = nil
- if !reflect.DeepEqual(&newc, wantc) {
- t.Errorf("conn: %+v, want %+v", &newc, wantc)
+
+ for _, tc := range testcases {
+ c, err := drv{}.Open(tc.connStr)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer c.Close()
+
+ wantc := tc.conn
+ newc := *(c.(*conn))
+ newc.Address = ""
+ newc.vtgateConn = nil
+ if !reflect.DeepEqual(&newc, wantc) {
+ t.Errorf("%v: conn: %+v, want %+v", tc.desc, &newc, wantc)
+ }
  }
 }
 
@@ -204,6 +218,46 @@ func TestExec(t *testing.T) {
  }
 }
 
+func TestConfigurationToJSON(t *testing.T) {
+ var testcases = []struct {
+ desc string
+ config Configuration
+ json string
+ }{
+ {
+ desc: "all fields set",
+ config: Configuration{
+ Protocol: "some-invalid-protocol",
+ Keyspace: "ks2",
+ Shard: "-80",
+ TabletType: "replica",
+ Streaming: true,
+ Timeout: 1 * time.Second,
+ },
+ json: `{"Protocol":"some-invalid-protocol","Address":"","Keyspace":"ks2","Shard":"-80","tablet_type":"replica","Streaming":true,"Timeout":1000000000}`,
+ },
+ {
+ desc: "default fields are empty",
+ config: Configuration{
+ Keyspace: "ks2",
+ Shard: "-80",
+ Timeout: 1 * time.Second,
+ },
+ json: `{"Protocol":"grpc","Address":"","Keyspace":"ks2","Shard":"-80","tablet_type":"master","Streaming":false,"Timeout":1000000000}`,
+ },
+ }
+
+ for _, tc := range testcases {
+ json, err := tc.config.toJSON()
+ if err != nil {
+ t.Errorf("%v: JSON conversion should have succeeded but did not: %v", tc.desc, err)
+ }
+ if json != tc.json {
+ t.Errorf("%v: Configuration.JSON(): got: %v want: %v Configuration: %v", tc.desc, json, tc.json, tc.config)
+ }
+ }
+}
+
 func TestExecStreamingNotAllowed(t *testing.T) {
  db, err := OpenForStreaming(testAddress, "rdonly", 30*time.Second)
  if err != nil {

go/vt/vtgate/vtgateconn/vtgateconn.go

@@ -23,7 +23,8 @@ const (
 )
 
 var (
- vtgateProtocol = flag.String("vtgate_protocol", GoRPCProtocol, "how to talk to vtgate")
+ // VtgateProtocol defines the RPC implementation used for connecting to vtgate.
+ VtgateProtocol = flag.String("vtgate_protocol", GoRPCProtocol, "how to talk to vtgate")
 )
 
 // VTGateConn is the client API object to talk to vtgate.
@@ -424,5 +425,5 @@ func DialProtocol(ctx context.Context, protocol string, address string, timeout
 // Dial dials using the command-line specified protocol, and returns
 // the *VTGateConn.
 func Dial(ctx context.Context, address string, timeout time.Duration) (*VTGateConn, error) {
- return DialProtocol(ctx, *vtgateProtocol, address, timeout)
+ return DialProtocol(ctx, *VtgateProtocol, address, timeout)
 }