Add protoutil package, refactor ISP to use it

.github/CODEOWNERS

@@ -7,6 +7,7 @@
 /go/cmd/vtadmin @ajm188 @doeg
 /go/cmd/vtctldclient @ajm188 @doeg
 /go/mysql @harshit-gangal @systay
+/go/protoutil @ajm188
 /go/test/endtoend/onlineddl @shlomi-noach
 /go/test/endtoend/orchestrator @deepthi @shlomi-noach
 /go/test/endtoend/vtgate @harshit-gangal @systay

go/protoutil/doc.go

@@ -0,0 +1,25 @@
+/*
+Copyright 2021 The Vitess Authors.
+
+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
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+/*
+Package protoutil provides helper functions for working with well-known protobuf
+types.
+
+It aims to serve a purpose similar to packages topoproto and mysqlctlproto, but
+for general, common types, rather than types related to a particular Vitess RPC
+service.
+*/
+package protoutil

go/protoutil/duration.go

@@ -0,0 +1,42 @@
+/*
+Copyright 2021 The Vitess Authors.
+
+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
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package protoutil
+
+import (
+ "time"
+
+ "github.com/golang/protobuf/ptypes"
+
+ durationpb "github.com/golang/protobuf/ptypes/duration"
+)
+
+// DurationFromProto converts a durationpb type to a time.Duration. It returns a
+// three-tuple of (dgo, ok, err) where dgo is the go time.Duration, ok indicates
+// whether the proto value was set, and err is set on failure to convert the
+// proto value.
+func DurationFromProto(dpb *durationpb.Duration) (time.Duration, bool, error) {
+ if dpb == nil {
+ return 0, false, nil
+ }
+
+ dgo, err := ptypes.Duration(dpb)
+ if err != nil {
+ return 0, true, err
+ }
+
+ return dgo, true, nil
+}

go/protoutil/duration_test.go

@@ -0,0 +1,83 @@
+/*
+Copyright 2021 The Vitess Authors.
+
+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
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package protoutil
+
+import (
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+
+ durationpb "github.com/golang/protobuf/ptypes/duration"
+)
+
+func TestDurationFromProto(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ in *durationpb.Duration
+ expected time.Duration
+ isOk bool
+ shouldErr bool
+ }{
+ {
+ name: "success",
+ in: &durationpb.Duration{Seconds: 1000},
+ expected: time.Second * 1000,
+ isOk: true,
+ shouldErr: false,
+ },
+ {
+ name: "nil value",
+ in: nil,
+ expected: 0,
+ isOk: false,
+ shouldErr: false,
+ },
+ {
+ name: "error",
+ in: &durationpb.Duration{
+ // This is the max allowed seconds for a durationpb, plus 1.
+ Seconds: int64(10000*365.25*24*60*60) + 1,
+ },
+ expected: 0,
+ isOk: true,
+ shouldErr: true,
+ },
+ }
+
+ for _, tt := range tests {
+ tt := tt
+
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+
+ actual, ok, err := DurationFromProto(tt.in)
+ if tt.shouldErr {
+ assert.Error(t, err)
+ assert.Equal(t, tt.isOk, ok, "expected (_, ok, _) = DurationFromProto; to be ok = %v", tt.isOk)
+
+ return
+ }
+
+ assert.NoError(t, err)
+ assert.Equal(t, tt.expected, actual)
+ assert.Equal(t, tt.isOk, ok, "expected (_, ok, _) = DurationFromProto; to be ok = %v", tt.isOk)
+ })
+ }
+}

go/vt/vtctl/grpcvtctldserver/server.go

@@ -23,10 +23,10 @@ import (
  "sync"
  "time"
 
- "github.com/golang/protobuf/ptypes"
  "google.golang.org/grpc"
 
  "vitess.io/vitess/go/event"
+ "vitess.io/vitess/go/protoutil"
  "vitess.io/vitess/go/sqlescape"
  "vitess.io/vitess/go/vt/concurrency"
  "vitess.io/vitess/go/vt/log"
@@ -365,15 +365,10 @@ func (s *VtctldServer) InitShardPrimary(ctx context.Context, req *vtctldatapb.In
  return nil, vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "shard field is required")
  }
 
- var waitReplicasTimeout time.Duration
- if req.WaitReplicasTimeout != nil {
- wrt, err := ptypes.Duration(req.WaitReplicasTimeout)
- if err != nil {
- return nil, vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "cannot parse WaitReplicasTimeout; err = %v", err)
- }
-
- waitReplicasTimeout = wrt
- } else {
+ waitReplicasTimeout, ok, err := protoutil.DurationFromProto(req.WaitReplicasTimeout)
+ if err != nil {
+ return nil, err
+ } else if !ok {
  waitReplicasTimeout = time.Second * 30
  }