Automatic validation

[vcraescu]

The entire entity validation logic should be reimplemented. I believe it's wrong how it works now because it makes almost impossible to use it at all. See the following scenario.

Controller

func (c *MyController) Edit(id string, frm *forms.MyForm) {
    var m models.MyModel

    c.findOr404(id, &m)

    if c.Req.Method == "POST" {
        // where i want to validate my form before save 
        }
}

This request will always fail even it is just a GET request and frm is supposed to be zero because nothing was submitted yet. Even If i remove any validation tags from forms.MyForm and put them directly on models.MyModel just to overcome this issue, but If forms.MyForm has any reference to a model which has validation tags again it will fail. Spent so much time fighting this behaviour.

Having a single HandleError method is a very bad idea in my opinion because when you end up in HandleError method your context is lost. You don't know the action where the error occurred and how you're supposed to handle it. You just end up with an error object you have to deal with but you don't know how. In the wild that's not always the case. Validation can be complex and might have a lot of logic behind and it needs to be very flexible. Magic kills flexibility. This might make sense If we would have single action controllers or single verb per action. But as longs as you have multiple verbs per action you gonna have a really bad day. :sad:

A better approach would be to remove any automatic validation and let the user handle it. I mean the context could have a method Validate(m interface{}) which is just a proxy to validator and let the programmer validate the objects he needs. Something like below:

func (c *MyController) Edit(id string, frm *forms.MyForm) {
    var m models.MyModel

    c.findOr404(id, &m)

    if c.Req.Method == "POST" {
            if formErrors := c.Validate(m); len(formErrors) > 0 {
            data["FormErrors"] = formErrors
            c.render(data)
            return
        }
                // save
        }
}

[jeevatkm]

@vcraescu Thanks for reporting an issue.

as longs as you have multiple verbs per action you gonna have a really bad day.

The scenario you have described, its seems you're having using same controller action for GET and POST multiple verbs. That could be a reason you could have faced a challenge with auto validation follow.

Begin said that, I'm sorry, you had to spend time much around this one. I'm disappointed since one of the goal of aah is to have features for aah users to get going fast and maintainable.

Thanks for bringing up, I would surely make validation flow flexible and extensible for every possible scenario.

Magic kills flexibility

One of the goal of aah is have "configuration driven, magic, extensible and flexible". So magic shouldn't block the aah users to have their own way.

---

Let's take a spin for what aah currently provides -

• Have ability to control error handling at various level and propagate to higher level if needed.

  • Controller error handler
  • Application error handler
  • Frameworks default error handler

• Has every possible error could happen in the request flow, ErrValidation is one of them

https://github.com/go-aah/aah/blob/b1a2826abfb3d2cbc1809686671374b7e9969284/error.go#L21-L39

• Handle error method at controller level called for all errors could happen in the request flow for that Controller. Handle error method has input an argument, that would provide an insights of an error.

https://github.com/go-aah/aah/blob/b1a2826abfb3d2cbc1809686671374b7e9969284/error.go#L213-L218

So from the context of validation, what would be error object values: reason as ErrValidation, default HTTP response code as 400 (StatusBadRequest), err.Data is validation errors.

• HandleError called within same request flow, so context is exists fully, its not last (no response written yet). aah user have complete control over what goes as a Response from method HandleError.

Let's take a spin for what aah currently missing -

• It missing Controller Action Name (value exists already, could be exposed)
• It missing Controller Name (value exists already, could be exposed)
• It missing Route Name (value exists already, could be exposed)

---

What are the possible solutions we could go for -

Solution 1:

• Expose currently missing values for aah users
• Add HandleError<ActionName>(err *aah.Error) bool takes precedence over controller level, possible to propagate it higher level like current flow
• Add config option to disable Auto validation at route level
• Add helper method IsValidationError() at context (not sure how much it would be helpful)

Solution 2:

• Expose currently missing values for aah users
• Add HandleError<ActionName>(err *aah.Error) bool takes precedence over controller level, possible to propagate it higher level like current flow
• Remove validation flow from aah error handling flow
• Do auto validation and store the error info at context
• Expose method IsValidationError() to check validation error happended
• Expose method ValidationErrors() to retrive list of validation errors

Solution 3: (this is similar as your proposed solution)

• Expose currently missing values for aah users
• Add HandleError<ActionName>(err *aah.Error) bool takes precedence over controller level, possible to propagate it higher level like current flow
• Remove auto validation flow from request life cycle & aah error handling flow

PS: Solution 3 have possibility of removing validation library from aah. It means aah user could have their choice validation library and performing validation at controller action.

[vcraescu]

@jeevatkm Yes, I'm using multiple verbs for an action. The app we're building right now It's just a prototype and we just render ol' school html. No REST, just plain html.

I'm not big fan of having a different method for validation because inside Action method you might have some logic to render the current action. Things which you will put into aah.Data object and pass it to the view. You usually do that before you actually validate the object because it is needed even the object is valid or not. If you're being moved to a different method to handle the error basically you will have to duplicate that logic there. Example:

func (c *MyController) Edit(id string, frm *forms.MyForm) {
    var m models.MyModel

    c.findOr404(id, &m)

        data := aah.Data{
                "SomeObjectsFromDB": someObjectsFromDB,
                "MyModel": m,
        }

    if c.Req.Method == "POST" {
            if formErrors := c.Validate(m); len(formErrors) > 0 {
            data["FormErrors"] = formErrors
            c.render(data)
            return
        }
                // save
               c.render(data)
               return
        }

        // more logic
        c.render(data)
}

If the object is not valid, inside HandleErrorEdit method I will have to do this part again before I add the errors to aah.Data object:

    var m models.MyModel

    c.findOr404(id, &m)

        data := aah.Data{
                "SomeObjectsFromDB": someObjectsFromDB,
                "MyModel": m,
        }

My solution:

• Expose currently missing values for aah users: ctx.Validate, ctx.ValidateValue
• Add config option to disable Auto validation at route level
• Add config option to disable Auto validation at app level
• Add HandleError<ActionName>(err *aah.Error) bool takes precedence over controller level, possible to propagate it higher level like current flow
• Keep the validator, as long as the user can choose not to use it. It's ok to have something already there. If you want to use something else, fine, it's up to you as long as the framework does not stay in your way.

[jeevatkm]

@vcraescu Sounds good to me. Once its ready I will ping you, so that you could try it out and share feedback.