Refactor Validate() function.

field.go

@@ -8,8 +8,8 @@ import (
 )
 
 var (
-	errInvalidField          = errors.New("field must be a pointer to struct")
-	errFieldValidationFailed = errors.New("field failed validation")
+	errInvalidField = errors.New("field must be a pointer to struct")
+	errInvalidValue = errors.New("field contains an invalid value")
 )
 
 // field parses the value provided for the field and validates it.
@@ -18,26 +18,31 @@ func field(f interface{}, fieldValue string) error {
 	if !field.IsPtr() {
 		return errInvalidField
 	}
-	if err := parse(field, fieldValue); err != nil {
+	i, err := field.Field("Field").Type(fields.Field{}).Addr()
+	if err != nil {
 		return err
 	}
-	v, err := value(field)
+	fieldField := i.(*fields.Field)
+	e, err := parse(field, fieldValue)
 	if err != nil {
 		return err
 	}
-	i, err := field.Field("Field").Type(fields.Field{}).Addr()
+	if e != nil {
+		fieldField.Error = e
+		return errInvalidValue
+	}
+	v, err := value(field)
 	if err != nil {
 		return err
 	}
-	fieldField := i.(*fields.Field)
 	for _, validator := range fieldField.Validators {
 		e, err := run(validator, v)
 		if err != nil {
 			return err
 		}
 		if e != nil {
 			fieldField.Error = e
-			return errFieldValidationFailed
+			return errInvalidValue
 		}
 	}
 	return nil

field_test.go

@@ -9,16 +9,13 @@ import (
 	"github.com/nathan-osman/go-reflectr"
 )
 
-type testFieldMissingParse struct{}
-
-type testFieldMissingValue struct{}
+type testFieldMissingField struct{}
 
-func (t *testFieldMissingValue) Parse(string) error { return nil }
+type testFieldMissingParse struct{ fields.Field }
 
-type testFieldMissingField struct{}
+type testFieldMissingValue struct{ fields.Field }
 
-func (t *testFieldMissingField) Parse(string) error { return nil }
-func (t *testFieldMissingField) Value() interface{} { return nil }
+func (t *testFieldMissingValue) Parse(string) error { return nil }
 
 type testFieldBadValidator struct{}
 
@@ -29,11 +26,11 @@ func TestField(t *testing.T) {
 		Error      error
 	}{
 		{F: struct{}{}, Error: errInvalidField},
+		{F: &testFieldMissingField{}, Error: reflectr.ErrFieldDoesNotExist},
 		{F: &testFieldMissingParse{}, Error: errParseFieldValue},
 		{F: &testFieldMissingValue{}, Error: errGetFieldValue},
-		{F: &testFieldMissingField{}, Error: reflectr.ErrFieldDoesNotExist},
 		{F: fields.NewString(&testFieldBadValidator{}), Error: errRunValidator},
-		{F: fields.NewString(&validators.NonEmpty{}), Error: errFieldValidationFailed},
+		{F: fields.NewString(&validators.NonEmpty{}), Error: errInvalidValue},
 		{F: fields.NewString(&validators.NonEmpty{}), FieldValue: "a"},
 	} {
 		if err := field(test.F, test.FieldValue); err != test.Error {

parse.go

@@ -7,18 +7,18 @@ import (
 )
 
 var (
-	errParseFieldValue = errors.New("unable to parse form value")
+	errParseFieldValue = errors.New("unable to parse field value")
 )
 
 // parse calls the Parse() method of the field with the provided value.
-// The return value is the error returned by the call to Parse().
-func parse(field *reflectr.StructMeta, fieldValue string) error {
+// The first return value is the error returned by the call to Parse().
+func parse(field *reflectr.StructMeta, fieldValue string) (error, error) {
 	r, err := field.
 		Method("Parse").
 		Returns(reflectr.ErrorType).
 		Call(fieldValue)
 	if err != nil {
-		return errParseFieldValue
+		return nil, errParseFieldValue
 	}
-	return interfaceToError(r[0])
+	return interfaceToError(r[0]), nil
 }

parse_test.go

@@ -1,25 +1,37 @@
 package ezform
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/nathan-osman/go-ezform/fields"
-
 	"github.com/nathan-osman/go-reflectr"
 )
 
-type testBadParseField struct{}
+var errParseBadValue = errors.New("test")
 
-func TestBadParse(t *testing.T) {
-	s := reflectr.Struct(&testBadParseField{})
-	if err := parse(s, ""); err != errParseFieldValue {
-		t.Fatalf("%v != %v", err, errParseFieldValue)
-	}
-}
+type testParseBadField struct{}
+
+type testParseBadValueField struct{}
+
+func (t *testParseBadValueField) Parse(v string) error { return errParseBadValue }
 
 func TestParse(t *testing.T) {
-	s := reflectr.Struct(fields.NewString())
-	if err := parse(s, ""); err != nil {
-		t.Fatal(err)
+	for _, test := range []struct {
+		Field      interface{}
+		ParseError error
+		Error      error
+	}{
+		{Field: &testParseBadField{}, Error: errParseFieldValue},
+		{Field: &testParseBadValueField{}, ParseError: errParseBadValue},
+		{Field: fields.NewString()},
+	} {
+		e, err := parse(reflectr.Struct(test.Field), "")
+		if err != test.Error {
+			t.Fatalf("%v != %v", err, test.Error)
+		}
+		if e != test.ParseError {
+			t.Fatalf("%v != %v", e, test.ParseError)
+		}
 	}
 }

validate.go

@@ -4,56 +4,38 @@ import (
 	"errors"
 	"net/http"
 
-	"github.com/nathan-osman/go-ezform/fields"
 	"github.com/nathan-osman/go-reflectr"
 )
 
 var (
-	errInvalidForm  = errors.New("form must be a pointer to struct")
-	errInvalidField = errors.New("field must be a pointer to struct")
+	// ErrInvalid indicates that the form failed validation.
+	ErrInvalid = errors.New("form failed validation")
+
+	errInvalidForm = errors.New("form must be a pointer to struct")
 )
 
 // Validate parses request data and validates it against the provided form.
-func Validate(r *http.Request, v interface{}) (bool, error) {
+func Validate(r *http.Request, v interface{}) error {
 	s := reflectr.Struct(v)
 	if !s.IsPtr() {
-		return false, errInvalidForm
+		return errInvalidForm
 	}
 	if err := s.Error(); err != nil {
-		return false, errInvalidForm
+		return errInvalidForm
 	}
 	validated := true
 	for _, fieldName := range s.Fields() {
-		var (
-			f, _       = s.Field(fieldName).Value()
-			field      = reflectr.Struct(f)
-			fieldValue = r.Form.Get(fieldName)
-		)
-		if !field.IsPtr() {
-			return false, errInvalidField
-		}
-		if err := parse(field, fieldValue); err != nil {
-			return false, err
-		}
-		v, err := value(field)
-		if err != nil {
-			return false, err
-		}
-		i, err := field.Field("Field").Type(fields.Field{}).Addr()
-		if err != nil {
-			return false, err
-		}
-		fieldField := i.(*fields.Field)
-		for _, validator := range fieldField.Validators {
-			e, err := run(validator, v)
-			if err != nil {
-				return false, err
-			}
-			if e != nil {
-				fieldField.Error = e
+		f, _ := s.Field(fieldName).Value()
+		if err := field(f, r.Form.Get(fieldName)); err != nil {
+			if err == errInvalidValue {
 				validated = false
+			} else {
+				return err
 			}
 		}
 	}
-	return validated, nil
+	if !validated {
+		return ErrInvalid
+	}
+	return nil
 }

validate_test.go

@@ -17,64 +17,55 @@ const (
 	boolVal = true
 )
 
-func createRequest(params url.Values) (*http.Request, error) {
-	r := httptest.NewRequest(
-		http.MethodPost,
-		"/",
-		strings.NewReader(params.Encode()),
-	)
-	r.Header.Add("Content-Type", "application/x-www-form-urlencoded")
-	if err := r.ParseForm(); err != nil {
-		return nil, err
-	}
-	return r, nil
-}
-
-type testBadValidateForm struct{}
-
-func TestBadValidate(t *testing.T) {
-	if _, err := Validate(nil, new(string)); err != errInvalidForm {
-		t.Fatalf("%v != %v", err, errInvalidForm)
-	}
-	if _, err := Validate(nil, testBadValidateForm{}); err != errInvalidForm {
-		t.Fatalf("%v != %v", err, errInvalidForm)
+var (
+	testParams = url.Values{
+		"StrField":  []string{strVal},
+		"IntField":  []string{fmt.Sprintf("%v", intVal)},
+		"BoolField": []string{fmt.Sprintf("%v", boolVal)},
 	}
-}
+)
 
 type testValidateForm struct {
 	StrField  *fields.String
 	IntField  *fields.Integer
 	BoolField *fields.Boolean
 }
 
-func TestValidate(t *testing.T) {
-	f := &testValidateForm{
+type testValidateBadForm struct {
+	Field int
+}
+
+func newTestValidateForm() *testValidateForm {
+	return &testValidateForm{
 		StrField:  fields.NewString(),
 		IntField:  fields.NewInteger(),
 		BoolField: fields.NewBoolean(),
 	}
-	r, err := createRequest(url.Values{
-		"StrField":  []string{strVal},
-		"IntField":  []string{fmt.Sprintf("%v", intVal)},
-		"BoolField": []string{fmt.Sprintf("%v", boolVal)},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	v, err := Validate(r, f)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if !v {
-		t.Fatal("validation unexpectedly failed")
-	}
-	if f.StrField.Value() != strVal {
-		t.Fatalf("%v != %v", f.StrField.Value(), strVal)
-	}
-	if f.IntField.Value() != intVal {
-		t.Fatalf("%v != %v", f.IntField.Value(), intVal)
-	}
-	if f.BoolField.Value() != boolVal {
-		t.Fatalf("%v != %v", f.BoolField.Value(), boolVal)
+}
+
+func TestValidate(t *testing.T) {
+	for _, test := range []struct {
+		Params url.Values
+		Form   interface{}
+		Error  error
+	}{
+		{Form: testValidateForm{}, Error: errInvalidForm},
+		{Form: new(string), Error: errInvalidForm},
+		{Form: newTestValidateForm(), Error: ErrInvalid},
+		{Form: &testValidateBadForm{}, Error: errInvalidField},
+		{Params: testParams, Form: newTestValidateForm()},
+	} {
+		r := httptest.NewRequest(
+			http.MethodPost,
+			"/",
+			strings.NewReader(test.Params.Encode()),
+		)
+		r.Header.Add("Content-Type", "application/x-www-form-urlencoded")
+		if err := r.ParseForm(); err != nil {
+			t.Fatal(err)
+		}
+		if err := Validate(r, test.Form); err != test.Error {
+			t.Fatalf("%v != %v", err, test.Error)
+		}
 	}
 }