diff --git a/external/epsearchast/v3/validating_visitor.go b/external/epsearchast/v3/validating_visitor.go index 7d38106..fc2079c 100644 --- a/external/epsearchast/v3/validating_visitor.go +++ b/external/epsearchast/v3/validating_visitor.go @@ -2,113 +2,142 @@ package epsearchast_v3 import ( "fmt" + "github.com/go-playground/validator/v10" "reflect" "strings" ) -type ValidatingVisitor struct { +type validatingVisitor struct { AllowedOperators map[string][]string ColumnAliases map[string]string Visitor AstVisitor + ValueValidators map[string]string } -var _ AstVisitor = (*ValidatingVisitor)(nil) +// use a single instance of Validate, it caches struct info +var validate *validator.Validate -func NewValidatingVisitor(visitor AstVisitor, allowedOps map[string][]string, aliases map[string]string) AstVisitor { - return &ValidatingVisitor{ +func init() { + validate = validator.New() +} + +var _ AstVisitor = (*validatingVisitor)(nil) + +func NewValidatingVisitor(visitor AstVisitor, allowedOps map[string][]string, aliases map[string]string, valueValidators map[string]string) (AstVisitor, error) { + + for k, v := range aliases { + if _, ok := allowedOps[v]; !ok { + return nil, fmt.Errorf("alias from `%s` to `%s` points to a field not in the allowed ops", k, v) + } + } + + for k, v := range valueValidators { + if _, ok := allowedOps[k]; !ok { + if target, ok := aliases[k]; ok { + // Supporting aliases for validators would be messy because it could be many to one. + return nil, fmt.Errorf("validator for field `%s` with type `%s` points to an alias of `%s` instead of the field", k, v, target) + } else { + return nil, fmt.Errorf("validator for field `%s` with type `%s` points to an unknown field", k, v) + } + + } + } + + return &validatingVisitor{ Visitor: visitor, AllowedOperators: allowedOps, ColumnAliases: aliases, - } + ValueValidators: valueValidators, + }, nil } -func (v *ValidatingVisitor) PreVisit() error { +func (v *validatingVisitor) PreVisit() error { return v.Visitor.PreVisit() } -func (v *ValidatingVisitor) PostVisit() error { +func (v *validatingVisitor) PostVisit() error { return v.Visitor.PostVisit() } -func (v *ValidatingVisitor) PreVisitAnd(astNode *AstNode) (bool, error) { +func (v *validatingVisitor) PreVisitAnd(astNode *AstNode) (bool, error) { return v.Visitor.PreVisitAnd(astNode) } -func (v *ValidatingVisitor) PostVisitAnd(astNode *AstNode) (bool, error) { +func (v *validatingVisitor) PostVisitAnd(astNode *AstNode) (bool, error) { return v.Visitor.PostVisitAnd(astNode) } -func (v *ValidatingVisitor) VisitIn(astNode *AstNode) (bool, error) { +func (v *validatingVisitor) VisitIn(astNode *AstNode) (bool, error) { fieldName := astNode.Args[0] - if _, err := v.isOperatorValidForField("in", fieldName); err != nil { + if _, err := v.validateFieldAndValue("in", fieldName, astNode.Args[1:]...); err != nil { return false, err } return v.Visitor.VisitIn(astNode) } -func (v *ValidatingVisitor) VisitEq(astNode *AstNode) (bool, error) { +func (v *validatingVisitor) VisitEq(astNode *AstNode) (bool, error) { fieldName := astNode.FirstArg - if _, err := v.isOperatorValidForField("eq", fieldName); err != nil { + if _, err := v.validateFieldAndValue("eq", fieldName, astNode.SecondArg); err != nil { return false, err } return v.Visitor.VisitEq(astNode) } -func (v *ValidatingVisitor) VisitLe(astNode *AstNode) (bool, error) { +func (v *validatingVisitor) VisitLe(astNode *AstNode) (bool, error) { fieldName := astNode.FirstArg - if _, err := v.isOperatorValidForField("le", fieldName); err != nil { + if _, err := v.validateFieldAndValue("le", fieldName, astNode.SecondArg); err != nil { return false, err } return v.Visitor.VisitLe(astNode) } -func (v *ValidatingVisitor) VisitLt(astNode *AstNode) (bool, error) { +func (v *validatingVisitor) VisitLt(astNode *AstNode) (bool, error) { fieldName := astNode.FirstArg - if _, err := v.isOperatorValidForField("lt", fieldName); err != nil { + if _, err := v.validateFieldAndValue("lt", fieldName, astNode.SecondArg); err != nil { return false, err } return v.Visitor.VisitLt(astNode) } -func (v *ValidatingVisitor) VisitGe(astNode *AstNode) (bool, error) { +func (v *validatingVisitor) VisitGe(astNode *AstNode) (bool, error) { fieldName := astNode.FirstArg - if _, err := v.isOperatorValidForField("ge", fieldName); err != nil { + if _, err := v.validateFieldAndValue("ge", fieldName, astNode.SecondArg); err != nil { return false, err } return v.Visitor.VisitGe(astNode) } -func (v *ValidatingVisitor) VisitGt(astNode *AstNode) (bool, error) { +func (v *validatingVisitor) VisitGt(astNode *AstNode) (bool, error) { fieldName := astNode.FirstArg - if _, err := v.isOperatorValidForField("gt", fieldName); err != nil { + if _, err := v.validateFieldAndValue("gt", fieldName, astNode.SecondArg); err != nil { return false, err } return v.Visitor.VisitGt(astNode) } -func (v *ValidatingVisitor) VisitLike(astNode *AstNode) (bool, error) { +func (v *validatingVisitor) VisitLike(astNode *AstNode) (bool, error) { fieldName := astNode.FirstArg - if _, err := v.isOperatorValidForField("like", fieldName); err != nil { + if _, err := v.validateFieldAndValue("like", fieldName, astNode.SecondArg); err != nil { return false, err } return v.Visitor.VisitLike(astNode) } -func (v *ValidatingVisitor) isOperatorValidForField(operator, requestField string) (bool, error) { +func (v *validatingVisitor) isOperatorValidForField(operator, requestField string) (bool, error) { canonicalField := requestField if realName, ok := v.ColumnAliases[requestField]; ok { @@ -127,3 +156,36 @@ func (v *ValidatingVisitor) isOperatorValidForField(operator, requestField strin return false, fmt.Errorf("unknown operator [%s] specified in search filter for field [%s], allowed operators are %v", strings.ToLower(operator), requestField, v.AllowedOperators[canonicalField]) } + +func (v *validatingVisitor) validateFieldAndValue(operator, requestField string, values ...string) (bool, error) { + + if _, err := v.isOperatorValidForField(operator, requestField); err != nil { + return false, err + } + + canonicalField := requestField + if realName, ok := v.ColumnAliases[requestField]; ok { + canonicalField = realName + } + + if vv, ok := v.ValueValidators[canonicalField]; ok { + for _, value := range values { + err := validate.Var(value, vv) + + if err != nil { + + if verrors, ok := err.(validator.ValidationErrors); ok { + if len(verrors) > 0 { + verror := verrors[0] + return false, fmt.Errorf("could not validate [%s] with [%s], value [%s] does not satisify requirement [%s]", requestField, operator, verror.Value(), verror.Tag()) + } + } + + return false, fmt.Errorf("could not validate [%s] with [%s] validation error: %w", requestField, value, err) + } + + } + } + + return true, nil +} diff --git a/external/epsearchast/v3/validating_visitor_test.go b/external/epsearchast/v3/validating_visitor_test.go index 1f6a673..533fbe4 100644 --- a/external/epsearchast/v3/validating_visitor_test.go +++ b/external/epsearchast/v3/validating_visitor_test.go @@ -34,7 +34,8 @@ func TestValidationCatchesInvalidOperatorForBinaryOperatorsForKnownField(t *test mockObj.On("PreVisit").Return(nil). On("PostVisit").Return(nil) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"amount": {otherBinOp}}, map[string]string{}) + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"amount": {otherBinOp}}, map[string]string{}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -68,7 +69,8 @@ func TestValidationCatchesInvalidOperatorForBinaryOperatorsForUnknownField(t *te mockObj.On("PreVisit").Return(nil). On("PostVisit").Return(nil) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"other_field": {otherBinOp}}, map[string]string{}) + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"other_field": {otherBinOp}}, map[string]string{}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -102,7 +104,8 @@ func TestValidationReturnsErrorForBinaryOperators(t *testing.T) { On("PostVisit").Return(nil). On(fmt.Sprintf("Visit%s", strings.Title(binOp)), mock.Anything).Return(true, fmt.Errorf("mocked error: %s", binOp)) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"amount": {binOp}}, map[string]string{}) + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"amount": {binOp}}, map[string]string{}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -135,7 +138,8 @@ func TestValidationReturnsErrorForBinaryOperatorsWithAlias(t *testing.T) { On("PostVisit").Return(nil). On(fmt.Sprintf("Visit%s", strings.Title(binOp)), mock.Anything).Return(true, fmt.Errorf("mocked error: %s", binOp)) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"total": {binOp}}, map[string]string{"amount": "total"}) + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"total": {binOp}}, map[string]string{"amount": "total"}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -146,6 +150,39 @@ func TestValidationReturnsErrorForBinaryOperatorsWithAlias(t *testing.T) { } } +func TestValidationReturnsErrorForBinaryOperatorsValueValidation(t *testing.T) { + + for _, binOp := range binOps { + t.Run(fmt.Sprintf("%s", binOp), func(t *testing.T) { + // Fixture Setup + // language=JSON + jsonTxt := fmt.Sprintf(` + { + "type": "%s", + "first_arg": "amount", + "second_arg": "5" + } + `, strings.ToUpper(binOp)) + + astNode, err := GetAst(jsonTxt) + require.NoError(t, err) + + mockObj := new(MyMockedVisitor) + mockObj.On("PreVisit").Return(nil). + On("PostVisit").Return(nil) + + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"total": {binOp}}, map[string]string{"amount": "total"}, map[string]string{"total": "uuid"}) + require.NoError(t, err) + + // Execute SUT + err = astNode.Accept(visitor) + + // Verification + require.ErrorContains(t, err, fmt.Sprintf("could not validate [amount] with [%s]", binOp)) + }) + } +} + func TestValidationCatchesInvalidOperatorForVariableOperatorsForKnownField(t *testing.T) { for idx, varOp := range varOps { t.Run(fmt.Sprintf("%s", varOp), func(t *testing.T) { @@ -167,8 +204,8 @@ func TestValidationCatchesInvalidOperatorForVariableOperatorsForKnownField(t *te mockObj.On("PreVisit").Return(nil). On("PostVisit").Return(nil) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"amount": {otherBinOp}}, map[string]string{}) - + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"amount": {otherBinOp}}, map[string]string{}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -200,7 +237,8 @@ func TestValidationCatchesInvalidOperatorForVariableOperatorsForUnknownField(t * mockObj.On("PreVisit").Return(nil). On("PostVisit").Return(nil) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"other_field": {otherBinOp}}, map[string]string{}) + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"other_field": {otherBinOp}}, map[string]string{}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -233,7 +271,8 @@ func TestValidationReturnsErrorForVariableOperators(t *testing.T) { On("PostVisit").Return(nil). On(fmt.Sprintf("Visit%s", strings.Title(varOp)), mock.Anything).Return(true, fmt.Errorf("mocked error: %s", varOp)) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"amount": {varOp}}, map[string]string{}) + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"amount": {varOp}}, map[string]string{}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -265,7 +304,8 @@ func TestValidationReturnsErrorForVariableOperatorsWithAlias(t *testing.T) { On("PostVisit").Return(nil). On(fmt.Sprintf("Visit%s", strings.Title(varOp)), mock.Anything).Return(true, fmt.Errorf("mocked error: %s", varOp)) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"total": {varOp}}, map[string]string{"amount": "total"}) + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"total": {varOp}}, map[string]string{"amount": "total"}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -276,6 +316,38 @@ func TestValidationReturnsErrorForVariableOperatorsWithAlias(t *testing.T) { } } +func TestValidationReturnsErrorForVariableOperatorsValueValidation(t *testing.T) { + + for _, varOp := range varOps { + t.Run(fmt.Sprintf("%s", varOp), func(t *testing.T) { + // Fixture Setup + // language=JSON + jsonTxt := fmt.Sprintf(` + { + "type": "%s", + "args": ["email", "foo@foo.com", "bar@bar.com", "5"] + } + `, strings.ToUpper(varOp)) + + astNode, err := GetAst(jsonTxt) + require.NoError(t, err) + + mockObj := new(MyMockedVisitor) + mockObj.On("PreVisit").Return(nil). + On("PostVisit").Return(nil) + + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"email": {varOp}}, map[string]string{}, map[string]string{"email": "email"}) + require.NoError(t, err) + + // Execute SUT + err = astNode.Accept(visitor) + + // Verification + require.ErrorContains(t, err, fmt.Sprintf("could not validate [email] with [%s]", varOp)) + }) + } +} + func TestValidationReturnsErrorForPostVisit(t *testing.T) { // Fixture Setup @@ -294,7 +366,8 @@ func TestValidationReturnsErrorForPostVisit(t *testing.T) { On("PostVisit").Return(fmt.Errorf("mocked error: PostVisit")). On("VisitIn", mock.Anything).Return(true, nil) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"amount": {"in"}}, map[string]string{}) + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"amount": {"in"}}, map[string]string{}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -333,7 +406,8 @@ func TestValidationReturnsErrorForPostVisitAnd(t *testing.T) { On("PreVisitAnd", mock.Anything).Return(true, nil). On("PostVisitAnd", mock.Anything).Return(false, fmt.Errorf("mocked error: PostVisitAnd")) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"amount": {"in"}, "status": {"eq"}}, map[string]string{}) + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"amount": {"in"}, "status": {"eq"}}, map[string]string{}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -369,7 +443,8 @@ func TestValidationReturnsErrorForPreVisitAnd(t *testing.T) { mockObj.On("PreVisit").Return(nil). On("PreVisitAnd", mock.Anything).Return(false, fmt.Errorf("mocked error: PreVisitAnd")) - visitor := NewValidatingVisitor(mockObj, map[string][]string{"amount": {"in"}, "status": {"eq"}}, map[string]string{}) + visitor, err := NewValidatingVisitor(mockObj, map[string][]string{"amount": {"in"}, "status": {"eq"}}, map[string]string{}, map[string]string{}) + require.NoError(t, err) // Execute SUT err = astNode.Accept(visitor) @@ -378,3 +453,36 @@ func TestValidationReturnsErrorForPreVisitAnd(t *testing.T) { require.ErrorContains(t, err, fmt.Sprintf("mocked error: PreVisitAnd")) } + +func TestNewConstructorDetectsUnknownAliasTarget(t *testing.T) { + // Fixture Setup + mockObj := new(MyMockedVisitor) + + // Execute SUT + _, err := NewValidatingVisitor(mockObj, map[string][]string{"status": {"eq"}}, map[string]string{"total": "amount"}, map[string]string{}) + + // Verification + require.ErrorContains(t, err, fmt.Sprintf("alias from `total` to `amount` points to a field not in the allowed ops")) +} + +func TestNewConstructorDetectsUnknownValueValidatorTarget(t *testing.T) { + // Fixture Setup + mockObj := new(MyMockedVisitor) + + // Execute SUT + _, err := NewValidatingVisitor(mockObj, map[string][]string{"status": {"eq"}}, map[string]string{}, map[string]string{"total": "int"}) + + // Verification + require.ErrorContains(t, err, fmt.Sprintf("validator for field `total` with type `int` points to an unknown field")) +} + +func TestNewConstructorDetectsAliasedValueValidatorTarget(t *testing.T) { + // Fixture Setup + mockObj := new(MyMockedVisitor) + + // Execute SUT + _, err := NewValidatingVisitor(mockObj, map[string][]string{"status": {"eq"}}, map[string]string{"state": "status"}, map[string]string{"state": "int"}) + + // Verification + require.ErrorContains(t, err, fmt.Sprintf("validator for field `state` with type `int` points to an alias of `status` instead of the field")) +} diff --git a/go.mod b/go.mod index d4864b0..124bff9 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,14 @@ require github.com/stretchr/testify v1.8.2 require ( github.com/davecgh/go-spew v1.1.1 // indirect + github.com/go-playground/locales v0.14.1 // indirect + github.com/go-playground/universal-translator v0.18.1 // indirect + github.com/go-playground/validator/v10 v10.12.0 // indirect + github.com/leodido/go-urn v1.2.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.5.0 // indirect + golang.org/x/crypto v0.7.0 // indirect + golang.org/x/sys v0.6.0 // indirect + golang.org/x/text v0.8.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 74f5036..d23ec36 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,17 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= +github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= +github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= +github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= +github.com/go-playground/validator/v10 v10.12.0 h1:E4gtWgxWxp8YSxExrQFv5BpCahla0PVF2oTTEYaWQGI= +github.com/go-playground/validator/v10 v10.12.0/go.mod h1:hCAPuzYvKdP33pxWa+2+6AIKXEKqjIUyqsNCtbsSJrA= +github.com/leodido/go-urn v1.2.2 h1:7z68G0FCGvDk646jz1AelTYNYWrTNm0bEcFAo147wt4= +github.com/leodido/go-urn v1.2.2/go.mod h1:kUaIbLZWttglzwNuG0pgsh5vuV6u2YcGBYz1hIPjtOQ= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/rwtodd/Go.Sed v0.0.0-20210816025313-55464686f9ef/go.mod h1:8AEUvGVi2uQ5b24BIhcr0GCcpd/RNAFWaN2CJFrWIIQ= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= @@ -11,6 +20,12 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +golang.org/x/crypto v0.7.0 h1:AvwMYaRytfdeVt3u6mLaxYtErKYjxA2OXjJ1HHq6t3A= +golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU= +golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/text v0.8.0 h1:57P1ETyNKtuIjB4SRd15iJxuhj8Gc416Y78H3qgMh68= +golang.org/x/text v0.8.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=