transport: Expose ValidateRequest

This exposes a new public API: ValidateRequest. This function may be used by
transport authors to validate that a `transport.Request` is valid.

This function DOES NOT validate that a `context.Context` has a timeout set on
it. That's a transport-level concern (especially because unary transports
don't have timeouts).

As for the internal APIs: The `ValidateUnary` and `ValidateOneway` functions
have been replaced with `ValidateUnaryContext` and `ValidateOnewayContext`.

CHANGELOG.md

@@ -19,6 +19,8 @@ v1.0.0-dev (unreleased)
     `dontNotify` argument.
 -   Added a `transport.BadRequestError` function to build errors which satisfy
     `transport.IsBadRequestError`.
+-   Added a `transport.ValidateRequest` function to validate
+    `transport.Request`s.
 
 
 v1.0.0-rc3 (2016-12-09)

api/transport/request.go

@@ -20,7 +20,11 @@
 
 package transport
 
-import "io"
+import (
+	"io"
+
+	"go.uber.org/yarpc/internal/errors"
+)
 
 // Request is the low level request representation.
 type Request struct {
@@ -60,3 +64,25 @@ type Request struct {
 
 // Encoding represents an encoding format for requests.
 type Encoding string
+
+// ValidateRequest validates the given request. An error is returned if the
+// request is invalid.
+func ValidateRequest(req *Request) error {
+	var missingParams []string
+	if req.Service == "" {
+		missingParams = append(missingParams, "service name")
+	}
+	if req.Procedure == "" {
+		missingParams = append(missingParams, "procedure")
+	}
+	if req.Caller == "" {
+		missingParams = append(missingParams, "caller name")
+	}
+	if req.Encoding == "" {
+		missingParams = append(missingParams, "encoding")
+	}
+	if len(missingParams) > 0 {
+		return errors.MissingParameters(missingParams)
+	}
+	return nil
+}

internal/request/validator_test.go → api/transport/request_test.go

@@ -18,14 +18,15 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-package request
+package transport_test
 
 import (
 	"context"
 	"testing"
 	"time"
 
 	"go.uber.org/yarpc/api/transport"
+	"go.uber.org/yarpc/internal/request"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -36,8 +37,7 @@ func TestValidator(t *testing.T) {
 		transportType transport.Type
 		ttl           time.Duration
 
-		wantErr     error
-		wantMessage string
+		wantErr string
 	}{
 		{
 			// No error
@@ -56,43 +56,31 @@ func TestValidator(t *testing.T) {
 				Service:   "service",
 				Procedure: "hello",
 			},
-			wantErr: missingParametersError{
-				Parameters: []string{"encoding"},
-			},
-			wantMessage: "missing encoding",
+			wantErr: "missing encoding",
 		},
 		{
 			req: &transport.Request{
 				Service:   "service",
 				Procedure: "hello",
 				Encoding:  "raw",
 			},
-			wantErr: missingParametersError{
-				Parameters: []string{"caller name"},
-			},
-			wantMessage: "missing caller name",
+			wantErr: "missing caller name",
 		},
 		{
 			req: &transport.Request{
 				Caller:    "caller",
 				Procedure: "hello",
 				Encoding:  "raw",
 			},
-			wantErr: missingParametersError{
-				Parameters: []string{"service name"},
-			},
-			wantMessage: "missing service name",
+			wantErr: "missing service name",
 		},
 		{
 			req: &transport.Request{
 				Caller:   "caller",
 				Service:  "service",
 				Encoding: "raw",
 			},
-			wantErr: missingParametersError{
-				Parameters: []string{"procedure"},
-			},
-			wantMessage: "missing procedure",
+			wantErr: "missing procedure",
 		},
 		{
 			req: &transport.Request{
@@ -102,30 +90,20 @@ func TestValidator(t *testing.T) {
 				Encoding:  "raw",
 			},
 			transportType: transport.Unary,
-			wantErr: missingParametersError{
-				Parameters: []string{"TTL"},
-			},
-			wantMessage: "missing TTL",
+			wantErr:       "missing TTL",
 		},
 		{
-			req: &transport.Request{},
-			wantErr: missingParametersError{
-				Parameters: []string{
-					"service name", "procedure", "caller name", "encoding",
-				},
-			},
-			wantMessage: "missing service name, procedure, caller name, and encoding",
+			req:     &transport.Request{},
+			wantErr: "missing service name, procedure, caller name, and encoding",
 		},
 	}
 
 	for _, tt := range tests {
-		v := Validator{Request: tt.req}
-
 		ctx := context.Background()
-		err := v.ValidateCommon(ctx)
+		err := transport.ValidateRequest(tt.req)
 
 		if err == nil && tt.transportType == transport.Oneway {
-			err = v.ValidateOneway(ctx)
+			err = request.ValidateOnewayContext(ctx)
 		} else if err == nil { // default to unary
 			var cancel func()
 
@@ -134,13 +112,12 @@ func TestValidator(t *testing.T) {
 				defer cancel()
 			}
 
-			err = v.ValidateUnary(ctx)
+			err = request.ValidateUnaryContext(ctx)
 		}
 
-		if tt.wantErr != nil {
-			assert.Equal(t, tt.wantErr, err)
-			if tt.wantMessage != "" && err != nil {
-				assert.Equal(t, tt.wantMessage, err.Error())
+		if tt.wantErr != "" {
+			if assert.Error(t, err) {
+				assert.Equal(t, tt.wantErr, err.Error())
 			}
 		} else {
 			assert.NoError(t, err)

internal/errors/params.go

@@ -0,0 +1,47 @@
+package errors
+
+import (
+	"fmt"
+	"strings"
+)
+
+// MissingParameters returns an error representing a failure to process a
+// request because it was missing required parameters.
+func MissingParameters(params []string) error {
+	if len(params) == 0 {
+		return nil
+	}
+
+	return missingParametersError{Parameters: params}
+}
+
+// missingParametersError is a failure to process a request because it was
+// missing required parameters.
+type missingParametersError struct {
+	// Names of the missing parameters.
+	//
+	// Precondition: len(Parameters) > 0
+	Parameters []string
+}
+
+func (e missingParametersError) AsHandlerError() HandlerError {
+	return HandlerBadRequestError(e)
+}
+
+func (e missingParametersError) Error() string {
+	s := "missing "
+	ps := e.Parameters
+	if len(ps) == 1 {
+		s += ps[0]
+		return s
+	}
+
+	if len(ps) == 2 {
+		s += fmt.Sprintf("%s and %s", ps[0], ps[1])
+		return s
+	}
+
+	s += strings.Join(ps[:len(ps)-1], ", ")
+	s += fmt.Sprintf(", and %s", ps[len(ps)-1])
+	return s
+}

internal/errors/params_test.go

@@ -0,0 +1,40 @@
+package errors
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMissingParameters(t *testing.T) {
+	tests := []struct {
+		params []string
+		want   string
+	}{
+		{},
+		{
+			[]string{"x"},
+			"missing x",
+		},
+		{
+			[]string{"x", "y"},
+			"missing x and y",
+		},
+		{
+			[]string{"x", "y", "z"},
+			"missing x, y, and z",
+		},
+	}
+
+	for _, tt := range tests {
+		err := MissingParameters(tt.params)
+		if tt.want != "" {
+			if assert.Error(t, err) {
+				assert.Equal(t, tt.want, err.Error())
+			}
+		} else {
+			assert.NoError(t, err)
+		}
+
+	}
+}