From fa109a09a8a2f4be7381e6e4f5f2968e80d6926d Mon Sep 17 00:00:00 2001 From: sunboyy Date: Sun, 14 Feb 2021 11:48:09 +0700 Subject: [PATCH] Improve error message --- codecov.yml | 4 +-- internal/mongo/errors.go | 41 ++++++++++++++-------- internal/mongo/errors_test.go | 36 +++++++++++++++++++ internal/mongo/generator.go | 6 ++-- internal/mongo/generator_test.go | 15 +++++--- internal/spec/errors.go | 59 +++++++++++++++++++++++++------- internal/spec/errors_test.go | 41 ++++++++++++++++++++++ internal/spec/models.go | 26 ++++++++++++++ internal/spec/models_test.go | 45 ++++++++++++++++++++++++ internal/spec/parser.go | 6 ++-- internal/spec/parser_test.go | 41 +++++++++++----------- internal/spec/query.go | 15 ++++---- 12 files changed, 269 insertions(+), 66 deletions(-) create mode 100644 internal/mongo/errors_test.go create mode 100644 internal/spec/errors_test.go create mode 100644 internal/spec/models_test.go diff --git a/codecov.yml b/codecov.yml index 4c5b5bb..1f0045b 100644 --- a/codecov.yml +++ b/codecov.yml @@ -2,8 +2,8 @@ coverage: status: project: default: - target: 80% - threshold: 4% + target: 85% + threshold: 3% patch: default: target: 75% diff --git a/internal/mongo/errors.go b/internal/mongo/errors.go index b9e1e8e..a5faa9b 100644 --- a/internal/mongo/errors.go +++ b/internal/mongo/errors.go @@ -1,20 +1,31 @@ package mongo -// GenerationError is an error from generating MongoDB repository -type GenerationError string +import ( + "fmt" +) -func (err GenerationError) Error() string { - switch err { - case OperationNotSupportedError: - return "operation not supported" - case BsonTagNotFoundError: - return "bson tag not found" - } - return string(err) +// NewOperationNotSupportedError creates operationNotSupportedError +func NewOperationNotSupportedError(operationName string) error { + return operationNotSupportedError{OperationName: operationName} } -// generation error constants -const ( - OperationNotSupportedError GenerationError = "ERROR_OPERATION_NOT_SUPPORTED" - BsonTagNotFoundError GenerationError = "ERROR_BSON_TAG_NOT_FOUND" -) +type operationNotSupportedError struct { + OperationName string +} + +func (err operationNotSupportedError) Error() string { + return fmt.Sprintf("operation '%s' not supported", err.OperationName) +} + +// NewBsonTagNotFoundError creates bsonTagNotFoundError +func NewBsonTagNotFoundError(fieldName string) error { + return bsonTagNotFoundError{FieldName: fieldName} +} + +type bsonTagNotFoundError struct { + FieldName string +} + +func (err bsonTagNotFoundError) Error() string { + return fmt.Sprintf("bson tag of field '%s' not found", err.FieldName) +} diff --git a/internal/mongo/errors_test.go b/internal/mongo/errors_test.go new file mode 100644 index 0000000..b3d5a61 --- /dev/null +++ b/internal/mongo/errors_test.go @@ -0,0 +1,36 @@ +package mongo_test + +import ( + "testing" + + "github.com/sunboyy/repogen/internal/mongo" +) + +type ErrorTestCase struct { + Name string + Error error + ExpectedString string +} + +func TestError(t *testing.T) { + testTable := []ErrorTestCase{ + { + Name: "OperationNotSupportedError", + Error: mongo.NewOperationNotSupportedError("Stub"), + ExpectedString: "operation 'Stub' not supported", + }, + { + Name: "BsonTagNotFoundError", + Error: mongo.NewBsonTagNotFoundError("AccessToken"), + ExpectedString: "bson tag of field 'AccessToken' not found", + }, + } + + for _, testCase := range testTable { + t.Run(testCase.Name, func(t *testing.T) { + if testCase.Error.Error() != testCase.ExpectedString { + t.Errorf("Expected = %v\nReceived = %v", testCase.ExpectedString, testCase.Error.Error()) + } + }) + } +} diff --git a/internal/mongo/generator.go b/internal/mongo/generator.go index 2a9526f..ded09dc 100644 --- a/internal/mongo/generator.go +++ b/internal/mongo/generator.go @@ -87,9 +87,9 @@ func (g RepositoryGenerator) generateMethodImplementation(methodSpec spec.Method return g.generateDeleteImplementation(operation) case spec.CountOperation: return g.generateCountImplementation(operation) + default: + return "", NewOperationNotSupportedError(operation.Name()) } - - return "", OperationNotSupportedError } func (g RepositoryGenerator) generateInsertImplementation(operation spec.InsertOperation) (string, error) { @@ -201,7 +201,7 @@ func (g RepositoryGenerator) bsonTagFromFieldName(fieldName string) (string, err bsonTag, ok := structField.Tags["bson"] if !ok { - return "", BsonTagNotFoundError + return "", NewBsonTagNotFoundError(fieldName) } return bsonTag[0], nil diff --git a/internal/mongo/generator_test.go b/internal/mongo/generator_test.go index 80e854d..9ed9a58 100644 --- a/internal/mongo/generator_test.go +++ b/internal/mongo/generator_test.go @@ -1453,6 +1453,13 @@ type GenerateMethodInvalidTestCase struct { ExpectedError error } +type StubOperation struct { +} + +func (o StubOperation) Name() string { + return "Stub" +} + func TestGenerateMethod_Invalid(t *testing.T) { testTable := []GenerateMethodInvalidTestCase{ { @@ -1467,9 +1474,9 @@ func TestGenerateMethod_Invalid(t *testing.T) { code.ArrayType{ContainedType: code.PointerType{ContainedType: code.SimpleType("UserModel")}}, code.SimpleType("error"), }, - Operation: "search", + Operation: StubOperation{}, }, - ExpectedError: mongo.OperationNotSupportedError, + ExpectedError: mongo.NewOperationNotSupportedError("Stub"), }, { Name: "bson tag not found in query", @@ -1492,7 +1499,7 @@ func TestGenerateMethod_Invalid(t *testing.T) { }, }, }, - ExpectedError: mongo.BsonTagNotFoundError, + ExpectedError: mongo.NewBsonTagNotFoundError("AccessToken"), }, { Name: "bson tag not found in update field", @@ -1519,7 +1526,7 @@ func TestGenerateMethod_Invalid(t *testing.T) { }, }, }, - ExpectedError: mongo.BsonTagNotFoundError, + ExpectedError: mongo.NewBsonTagNotFoundError("AccessToken"), }, } diff --git a/internal/spec/errors.go b/internal/spec/errors.go index 5d6ac52..072329c 100644 --- a/internal/spec/errors.go +++ b/internal/spec/errors.go @@ -1,38 +1,73 @@ package spec +import ( + "fmt" + "strings" +) + // ParsingError is an error from parsing interface methods type ParsingError string func (err ParsingError) Error() string { switch err { - case UnknownOperationError: - return "unknown operation" - case UnsupportedNameError: - return "method name is not supported" case UnsupportedReturnError: return "this type of return is not supported" - case InvalidQueryError: - return "invalid query" + case QueryRequiredError: + return "query is required" case InvalidParamError: return "parameters do not match the query" case InvalidUpdateFieldsError: return "update fields is invalid" case ContextParamRequiredError: return "context parameter is required" - case StructFieldNotFoundError: - return "struct field not found" } return string(err) } // parsing error constants const ( - UnknownOperationError ParsingError = "ERROR_UNKNOWN_OPERATION" - UnsupportedNameError ParsingError = "ERROR_UNSUPPORTED" UnsupportedReturnError ParsingError = "ERROR_UNSUPPORTED_RETURN" - InvalidQueryError ParsingError = "ERROR_INVALID_QUERY" + QueryRequiredError ParsingError = "ERROR_QUERY_REQUIRED" InvalidParamError ParsingError = "ERROR_INVALID_PARAM" InvalidUpdateFieldsError ParsingError = "ERROR_INVALID_UPDATE_FIELDS" ContextParamRequiredError ParsingError = "ERROR_CONTEXT_PARAM_REQUIRED" - StructFieldNotFoundError ParsingError = "ERROR_STRUCT_FIELD_NOT_FOUND" ) + +// NewInvalidQueryError creates invalidQueryError +func NewInvalidQueryError(queryTokens []string) error { + return invalidQueryError{QueryString: strings.Join(queryTokens, "")} +} + +type invalidQueryError struct { + QueryString string +} + +func (err invalidQueryError) Error() string { + return fmt.Sprintf("invalid query '%s'", err.QueryString) +} + +// NewUnknownOperationError creates unknownOperationError +func NewUnknownOperationError(operationName string) error { + return unknownOperationError{OperationName: operationName} +} + +type unknownOperationError struct { + OperationName string +} + +func (err unknownOperationError) Error() string { + return fmt.Sprintf("unknown operation '%s'", err.OperationName) +} + +// NewStructFieldNotFoundError creates structFieldNotFoundError +func NewStructFieldNotFoundError(fieldName string) error { + return structFieldNotFoundError{FieldName: fieldName} +} + +type structFieldNotFoundError struct { + FieldName string +} + +func (err structFieldNotFoundError) Error() string { + return fmt.Sprintf("struct field '%s' not found", err.FieldName) +} diff --git a/internal/spec/errors_test.go b/internal/spec/errors_test.go new file mode 100644 index 0000000..e3dfe0e --- /dev/null +++ b/internal/spec/errors_test.go @@ -0,0 +1,41 @@ +package spec_test + +import ( + "testing" + + "github.com/sunboyy/repogen/internal/spec" +) + +type ErrorTestCase struct { + Name string + Error error + ExpectedString string +} + +func TestError(t *testing.T) { + testTable := []ErrorTestCase{ + { + Name: "UnknownOperationError", + Error: spec.NewUnknownOperationError("Search"), + ExpectedString: "unknown operation 'Search'", + }, + { + Name: "StructFieldNotFoundError", + Error: spec.NewStructFieldNotFoundError("Country"), + ExpectedString: "struct field 'Country' not found", + }, + { + Name: "InvalidQueryError", + Error: spec.NewInvalidQueryError([]string{"By", "And"}), + ExpectedString: "invalid query 'ByAnd'", + }, + } + + for _, testCase := range testTable { + t.Run(testCase.Name, func(t *testing.T) { + if testCase.Error.Error() != testCase.ExpectedString { + t.Errorf("Expected = %v\nReceived = %v", testCase.ExpectedString, testCase.Error.Error()) + } + }) + } +} diff --git a/internal/spec/models.go b/internal/spec/models.go index 9e635be..5880620 100644 --- a/internal/spec/models.go +++ b/internal/spec/models.go @@ -23,6 +23,7 @@ type MethodSpec struct { // Operation is an interface for any kind of operation type Operation interface { + Name() string } // InsertOperation is a method specification for insert operations @@ -30,12 +31,22 @@ type InsertOperation struct { Mode QueryMode } +// Name returns "Insert" operation name +func (o InsertOperation) Name() string { + return "Insert" +} + // FindOperation is a method specification for find operations type FindOperation struct { Mode QueryMode Query QuerySpec } +// Name returns "Find" operation name +func (o FindOperation) Name() string { + return "Find" +} + // UpdateOperation is a method specification for update operations type UpdateOperation struct { Fields []UpdateField @@ -43,6 +54,11 @@ type UpdateOperation struct { Query QuerySpec } +// Name returns "Update" operation name +func (o UpdateOperation) Name() string { + return "Update" +} + // UpdateField stores mapping between field name in the model and the parameter index type UpdateField struct { Name string @@ -55,7 +71,17 @@ type DeleteOperation struct { Query QuerySpec } +// Name returns "Delete" operation name +func (o DeleteOperation) Name() string { + return "Delete" +} + // CountOperation is a method specification for count operations type CountOperation struct { Query QuerySpec } + +// Name returns "Count" operation name +func (o CountOperation) Name() string { + return "Count" +} diff --git a/internal/spec/models_test.go b/internal/spec/models_test.go new file mode 100644 index 0000000..ba56c14 --- /dev/null +++ b/internal/spec/models_test.go @@ -0,0 +1,45 @@ +package spec_test + +import ( + "testing" + + "github.com/sunboyy/repogen/internal/spec" +) + +type OperationTestCase struct { + Operation spec.Operation + ExpectedName string +} + +func TestOperationName(t *testing.T) { + testTable := []OperationTestCase{ + { + Operation: spec.InsertOperation{}, + ExpectedName: "Insert", + }, + { + Operation: spec.FindOperation{}, + ExpectedName: "Find", + }, + { + Operation: spec.UpdateOperation{}, + ExpectedName: "Update", + }, + { + Operation: spec.DeleteOperation{}, + ExpectedName: "Delete", + }, + { + Operation: spec.CountOperation{}, + ExpectedName: "Count", + }, + } + + for _, testCase := range testTable { + t.Run(testCase.ExpectedName, func(t *testing.T) { + if testCase.Operation.Name() != testCase.ExpectedName { + t.Errorf("Expected = %v\nReceived = %v", testCase.ExpectedName, testCase.Operation.Name()) + } + }) + } +} diff --git a/internal/spec/parser.go b/internal/spec/parser.go index 6a541a1..9e70bde 100644 --- a/internal/spec/parser.go +++ b/internal/spec/parser.go @@ -48,7 +48,7 @@ func (p interfaceMethodParser) parseMethod() (Operation, error) { case "Count": return p.parseCountOperation(methodNameTokens[1:]) } - return nil, UnknownOperationError + return nil, NewUnknownOperationError(methodNameTokens[0]) } func (p interfaceMethodParser) parseInsertOperation(tokens []string) (Operation, error) { @@ -202,7 +202,7 @@ func (p interfaceMethodParser) parseUpdateOperation(tokens []string) (Operation, for _, field := range fields { structField, ok := p.StructModel.Fields.ByName(field.Name) if !ok { - return nil, StructFieldNotFoundError + return nil, NewStructFieldNotFoundError(field.Name) } if structField.Type != p.Method.Params[field.ParamIndex].Type { @@ -340,7 +340,7 @@ func (p interfaceMethodParser) validateQueryFromParams(params []code.Param, quer for _, predicate := range querySpec.Predicates { structField, ok := p.StructModel.Fields.ByName(predicate.Field) if !ok { - return StructFieldNotFoundError + return NewStructFieldNotFoundError(predicate.Field) } for i := 0; i < predicate.Comparator.NumberOfArguments(); i++ { diff --git a/internal/spec/parser_test.go b/internal/spec/parser_test.go index f557bb5..06bbeb2 100644 --- a/internal/spec/parser_test.go +++ b/internal/spec/parser_test.go @@ -850,8 +850,9 @@ func TestParseInterfaceMethod_Invalid(t *testing.T) { Name: "SearchByID", }) - if err != spec.UnknownOperationError { - t.Errorf("\nExpected = %v\nReceived = %v", spec.UnknownOperationError, err) + expectedError := spec.NewUnknownOperationError("Search") + if err != expectedError { + t.Errorf("\nExpected = %v\nReceived = %v", expectedError, err) } } @@ -1008,7 +1009,7 @@ func TestParseInterfaceMethod_Find_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.QueryRequiredError, }, { Name: "misplaced operator token (leftmost)", @@ -1019,7 +1020,7 @@ func TestParseInterfaceMethod_Find_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.NewInvalidQueryError([]string{"By", "And", "Gender"}), }, { Name: "misplaced operator token (rightmost)", @@ -1030,7 +1031,7 @@ func TestParseInterfaceMethod_Find_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.NewInvalidQueryError([]string{"By", "Gender", "And"}), }, { Name: "misplaced operator token (double operator)", @@ -1041,7 +1042,7 @@ func TestParseInterfaceMethod_Find_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.NewInvalidQueryError([]string{"By", "Gender", "And", "And", "City"}), }, { Name: "ambiguous query", @@ -1052,7 +1053,7 @@ func TestParseInterfaceMethod_Find_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.NewInvalidQueryError([]string{"By", "Gender", "And", "City", "Or", "Age"}), }, { Name: "no context parameter", @@ -1097,7 +1098,7 @@ func TestParseInterfaceMethod_Find_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.StructFieldNotFoundError, + ExpectedError: spec.NewStructFieldNotFoundError("Country"), }, { Name: "mismatched method parameter type", @@ -1209,7 +1210,7 @@ func TestParseInterfaceMethod_Update_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.QueryRequiredError, }, { Name: "ambiguous query", @@ -1220,7 +1221,7 @@ func TestParseInterfaceMethod_Update_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.NewInvalidQueryError([]string{"By", "ID", "And", "Username", "Or", "Gender"}), }, { Name: "no context parameter", @@ -1251,7 +1252,7 @@ func TestParseInterfaceMethod_Update_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.StructFieldNotFoundError, + ExpectedError: spec.NewStructFieldNotFoundError("Country"), }, { Name: "struct field does not match parameter in update fields", @@ -1343,7 +1344,7 @@ func TestParseInterfaceMethod_Delete_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.QueryRequiredError, }, { Name: "misplaced operator token (leftmost)", @@ -1354,7 +1355,7 @@ func TestParseInterfaceMethod_Delete_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.NewInvalidQueryError([]string{"By", "And", "Gender"}), }, { Name: "misplaced operator token (rightmost)", @@ -1365,7 +1366,7 @@ func TestParseInterfaceMethod_Delete_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.NewInvalidQueryError([]string{"By", "Gender", "And"}), }, { Name: "misplaced operator token (double operator)", @@ -1376,7 +1377,7 @@ func TestParseInterfaceMethod_Delete_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.NewInvalidQueryError([]string{"By", "Gender", "And", "And", "City"}), }, { Name: "ambiguous query", @@ -1387,7 +1388,7 @@ func TestParseInterfaceMethod_Delete_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.NewInvalidQueryError([]string{"By", "Gender", "And", "City", "Or", "Age"}), }, { Name: "no context parameter", @@ -1432,7 +1433,7 @@ func TestParseInterfaceMethod_Delete_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.StructFieldNotFoundError, + ExpectedError: spec.NewStructFieldNotFoundError("Country"), }, { Name: "mismatched method parameter type", @@ -1522,7 +1523,7 @@ func TestParseInterfaceMethod_Count_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.QueryRequiredError, }, { Name: "invalid query", @@ -1536,7 +1537,7 @@ func TestParseInterfaceMethod_Count_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.InvalidQueryError, + ExpectedError: spec.NewInvalidQueryError([]string{"By"}), }, { Name: "context parameter not provided", @@ -1593,7 +1594,7 @@ func TestParseInterfaceMethod_Count_Invalid(t *testing.T) { code.SimpleType("error"), }, }, - ExpectedError: spec.StructFieldNotFoundError, + ExpectedError: spec.NewStructFieldNotFoundError("Country"), }, } diff --git a/internal/spec/query.go b/internal/spec/query.go index 8e5f7da..b8eb1f0 100644 --- a/internal/spec/query.go +++ b/internal/spec/query.go @@ -95,11 +95,12 @@ func (t predicateToken) ToPredicate(paramIndex int) Predicate { return Predicate{Field: strings.Join(t, ""), Comparator: ComparatorEqual, ParamIndex: paramIndex} } -func parseQuery(tokens []string, paramIndex int) (QuerySpec, error) { - if len(tokens) == 0 { - return QuerySpec{}, InvalidQueryError +func parseQuery(rawTokens []string, paramIndex int) (QuerySpec, error) { + if len(rawTokens) == 0 { + return QuerySpec{}, QueryRequiredError } + tokens := rawTokens if len(tokens) == 1 && tokens[0] == "All" { return QuerySpec{}, nil } @@ -112,7 +113,7 @@ func parseQuery(tokens []string, paramIndex int) (QuerySpec, error) { } if len(tokens) == 0 || tokens[0] == "And" || tokens[0] == "Or" { - return QuerySpec{}, InvalidQueryError + return QuerySpec{}, NewInvalidQueryError(rawTokens) } var operator Operator @@ -122,7 +123,7 @@ func parseQuery(tokens []string, paramIndex int) (QuerySpec, error) { if token != "And" && token != "Or" { aggregatedToken = append(aggregatedToken, token) } else if len(aggregatedToken) == 0 { - return QuerySpec{}, InvalidQueryError + return QuerySpec{}, NewInvalidQueryError(rawTokens) } else if token == "And" && operator != OperatorOr { operator = OperatorAnd predicate := aggregatedToken.ToPredicate(paramIndex) @@ -136,11 +137,11 @@ func parseQuery(tokens []string, paramIndex int) (QuerySpec, error) { paramIndex += predicate.Comparator.NumberOfArguments() aggregatedToken = predicateToken{} } else { - return QuerySpec{}, InvalidQueryError + return QuerySpec{}, NewInvalidQueryError(rawTokens) } } if len(aggregatedToken) == 0 { - return QuerySpec{}, InvalidQueryError + return QuerySpec{}, NewInvalidQueryError(rawTokens) } predicates = append(predicates, aggregatedToken.ToPredicate(paramIndex))