Params validations

[jodosha]

Right now, the params passed to #call are an instance of Lotus::Action::Params which is a nice wrapper around a Ruby Hash.

I would love to pass subclasses of Params which are aware of the specific use case and they should be able to do:

• White label filtering: only the declared params will be available in the final object passed to #call.
• Coercions (optional feature): coerce a single param to the given Ruby class.
• Validations: if the given params aren't valid, halt with an HTTP status.

This approach has the following advantages:

• Avoid the need for low level entities to have mass assignment protection features.
• Save CPU cycles in case of validation failures: there is no need to hit lower layers if the data isn't valid.

We probably need to add this interface:

class Lotus::Action::Params
  def self.param(name, type, options = {})
    # ...
  end

  def initialize(params)
    @params = params
  end

  def validate!
    halt 412 unless valid?
  end

  def valid?
    # ...
  end
end

And implement one of those solutions:

A

class Signup
  include Lotus::Action
  params SignupParams

  def call(params)
    puts params.class # => SignupParams
  end
end

class SignupParams < Lotus::Action::Params
  param :first_name, String, presence: true
  param :last_name,  String, presence: true
  param :email,      String, presence: true, format: /(.*)/
end

Where:

def self.params(params_class)
  before do |params|
    params_class.new(params).validate!
  end
end

B

class Signup
  include Lotus::Action

  params do
    param :first_name, String, presence: true
    param :last_name,  String, presence: true
    param :email,      String, presence: true, format: /(.*)/
  end

  def call(params)
    puts params.class # => Signup::Params
  end
end

Where .params would generate an inner class of the action Signup::Params, which inherits from Lotus::Action::Params.

[fuadsaud]

Also it just occurred me: should we consider using decorators instead of subclasses of Params for this purpose? I'm thinking about composing params objects that deal with only a portion of the request. Though that could also be done through, mixins I guess:

class SomeAction
  def call(params)
    if some_condition_that_may_switch_the_parameter_format?
      params.extend(SomeParamsRule)
    else
      params.extend(AnotherParamsRule)
    end
  end
end

Just throwing out some ideas :)

PS.: I think you meant include Lotus::Action in the last example.

[jodosha]

@fuadsaud extending roles at the runtime is always a 👎 for me. Please read: http://tonyarcieri.com/dci-in-ruby-is-completely-broken

What's the use case for that? The main goal of standalone actions is to reduce conditionals, by mapping use case to action in a 1:1 relationship.

I'd love to hear more about this.

[AlfonsoUceda]

I think the Option A is better becAuse can be reusable, what do you think?

[fuadsaud]

So, about dynamically extending objects: I know it is slower than plain decorators (I think the comparison may in the post is a little unfair because he compares a case where he extends the object with one where no extension happens at all, which doesn't solve the problem in question), and I really think decorators are best fit for tasks like this (I just pointed out it was possible to do that through mixins).

About the params validation process itself: I guess you're right, it's better to encourage a more straightforward approach. I still think having the ability to compose different rules may be needed (I'm thinking about localized input like currencies, but I don't have a clear picture in my head of how that could work, I have to think more about that yet).

[stsc3000]

I am giving this a try (option A for now) - how should we validate nested hashes?
I think something like this should do

param :user do
  param :name, String, presence: true
  param :email, String, format: /.../
end

It should also be possible to validate whether each item in a given array fits the given validations, although I am not sure how the syntax for that would look.
The following seems reasonable:

param :a_bunch_of_names, String, each: { format: /.../ }

What do you think?

[jodosha]

@AlfonsoUceda I was thinking of having a Lotus::Validations mixin so you can still achieve reusability with Option B.

module MyCommonValidations
  include Lotus::Validations

  param :email, String, presence: true, format: /(.*)/
end

class Signup
  include Lotus::Action

  params do
    extend MyCommonValidations
    param :first_name, String, presence: true
    param :last_name,  String, presence: true
  end

  def call(params)
    puts params.class # => Signup::Params
  end
end

[jodosha]

@stsc3000 what if we do

param :a_bunch_of_names, validator: MyValidator

[stsc3000]

@jodosha I like that - this way a user can provide a custom MyValidator instead of relying on the common presence, format, length etc validations which would be included by default - if that is what you mean. I don't know whether an array in params is uncommon enough to warrant a custom Validator implementation by the user. Maybe another method instead of param is a pragmatic solution.

[jodosha]

@stsc3000 I assume that MyValidator includes Lotus::Validations, as shown in the examples above.

[vitaLee]

@jodosha
What about a way to supply the custom validator with some data used in the validation process?
For example recently I experimented with params validator object, that could be supplied with validation rules, each one validating some part of the params.
The validation rules in my case could be any object responding to call, and I used to configure the validator with validation rules and call validate! on it in a before callback.
So in the end I had something like this for a validation rule

class AttachmentBelongsToSite
  def initialize(site)
    @site = site
  end

  def call(params)
    @site.attachments.exists? params[:attachment_id]
  end
end

and have this in controller

# before_action callback
def validate_params
  validator = ParamsValidator.new
  validator.add_rule AttachmentBelongsToSite.new(current_site)
  validator.validate!
end

[jodosha]

@vitaLee I like it. How do you use it?

[vitaLee]

Oops I forgot this important detail, passing params when calling validator.validate!.

def validate_params
  validator = ParamsValidator.new
  validator.add_rule AttachmentBelongsToSite.new(current_site)
  validator.validate!(params)
end

I didn't make it clear but in case it didn't become clear, we're talking about Rails app here.
So I use like this

# controller
rescue_from ParamsValidator::InvalidParamsError, with: :invalid_params

def invalid_params
  head :unprocessable_entity
end

ParamsValidator is iterating all validation rules it was supplied with and raises ParamsValidator::InvalidParamsError for the rule that returns false.

[jodosha]

@vitaLee why do you add rules at the runtime instead of having a concrete implementation at the beginning?

[vitaLee]

@jodosha In my case I had a base controller that supplied the validator with shared validation rules and allowed subcontrollers to append additional rules like this

def validate_params
  validator = ParamsValidator.new
  validator.add_rule SharedValidationRule.new
  validator.add_rule AnotherSharedValidationRule.new

  configure_params_validator validator # allow subcontroller to add additional rules

  validator.validate!(params)
end

def configure_params_validator(validator)
   # noop in base controller but in subcontrollers we could append additional rules
end

[vitaLee]

why do you add rules at the runtime instead of having a concrete implementation at the beginning?

this way I can feed the validator with rules constructed with data specific to the context of the current request, and it's more transparent what rules I use in each subcontroller.
Have in mind I didn't play with the idea a lot, but what I liked about it is that

• rules could be as simple as proc objects and more complex custom classes.
• rules can be reused in many places

In my case I just didn't hit the point where I felt the need to subclass ParamsValidator to have it preconfigured with some rules.

[mjbellantoni]

I've implemented the whitelisting portion of this in pull #31. It implements it as laid out in the "A" proposal. If I get the thumbs up on this part, I'll move on to the the coercion and validation parts.

[jodosha]

One unanswered question is: where this class of option "A" should be placed? Eg. apps/bookshelf/params/signup/create.rb? It seems a bit overkilling.

My opinion is: we should use @mjbellantoni work on #31 (which implements A) and have option B as default. So that, we don't have to find a default place for concrete params classes. But developers who want to do otherwise can use .params MyParamsClass.

[mjbellantoni]

I don't think it will be that hard to support both options, actually. I've almost got type coercions completed. From there I'll either move onto validations or DSL support as laid out in option "B".

[jodosha]

@mjbellantoni I'd like to have validations as a separated mixin. Please keep this in mind while design. I'm thinking to a Lotus::Validations gem ;)

[viking]

For supplying data to validations (such as when checking for a unique value in a collection of entities, for example), it seems like you would have to have a way to pass a scope to the validator, like @vitaLee mentioned. What if controller exposures were made available to a validation object, similar to Lotus::View?

[viking]

Maybe something like this: hanami/validations#2

[jodosha]

@viking Uniqueness validators are a lie.
You can't ensure uniqueness at application level, only with unique indexes at the database level. For this reason this feature will not be part of Lotus::Validators.

For a long technical explanation, please read: http://robots.thoughtbot.com/the-perils-of-uniqueness-validations

[viking]

Fair enough.

On August 5, 2014 5:01:01 PM CDT, Luca Guidi notifications@github.com wrote:

@viking Uniqueness validators are a lie.
You can't ensure uniqueness at application level, only with unique
indexes at the database level. For this reason this feature will not be
part of Lotus::Validators.

For a long technical explanation, please read:
http://robots.thoughtbot.com/the-perils-of-uniqueness-validations

---

Reply to this email directly or view it on GitHub:
#22 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[sarahhenkens]

@jodosha The article is great on explaining what might go wrong. But what about a solution? Imagine a user model with a username and email field, both of these need to be unique.

I do a POST and submit duplicate content for both fields. Relying on constraint exceptions will only trigger the first found constraint violation. Resulting in only it invalidating the first field. The user has to submit it again with the modified username to get the second validation constraint.

Why not both? Add a uniqueness check in the validations block and also handle constraint violations properly in case the race condition DOES happen.

[beauby]

@sarahhenkens I believe you meant to comment on #232.

[jodosha]

@sarahhenkens Thanks for your input. 😄 Given the schema of a validation is totally decoupled from any entity/repository/database. How could be the syntax to indicate the uniqueness of a given field?

params do
  required(:user).schema do
    required(:email).filled(:str?)
  end
end

In the case above :user maps the expected params, not an entity.

[sarahhenkens]

The way I've implemented it in my project at the moment is:

  class Validator
    include Hanami::Validations

    predicate :unique?, message: "must be unique" do |field, current|
      User.where(field => current).count == 0
    end

    predicate :email?, message: "must be an email" do |current|
      current =~ /\A([\w+\-].?)+@[a-z\d\-]+(\.[a-z]+)*\.[a-z]+\z/i
    end

    validations do
      required(:email).filled(:str?, :email?) { unique?(:email) }
      required(:username).filled(:str?) { unique?(:username) }
      required(:password).filled(:str?)
    end
  end

I've added a unique? predicate that is currently coupled to my model layer. The model's column is a parameter of the predicate. Decoupling it from the input param naming conventions.

[beauby]

I've been doing roughly the same as @sarahhenkens.

[jodosha]

@sarahhenkens @beauby yup. Is your a proposal to have a standard predicate like that included in hanami, or are you suggesting to use that snippet above as example in docs?

[beauby]

@jodosha I'd be in favor of an example in the docs, as providing a standard predicate would force too many assumptions. My main concern was that in #232, the discussion seemed to imply that the maintainers would label uniqueness validations as "bad practice", whereas it is kind of unavoidable in certain cases.

[jodosha]

@beauby the only problem that I have by adding to the guides is that everything documented there will eventually become the "Hanami way". Honestly I won't push for that uniqueness validation.

WDYT?

[beauby]

@jodosha I may be missing something but I don't see any other solution than uniqueness validations for informing the user about which field broke a uniqueness constraint (except when there's only one). Hence, I'm advocating for uniqueness validations to become the "Hanami way". FWIW, I remember discussing this with @solnic who held the same views.

[AlfonsoUceda]

@beauby @jodosha we can document the uniqueness validation but this validation will have to be implemented by the user because it will be coupled with the repository. This is the only validation I see, we can suggest to be coupled with a repository. what do you think?

[jodosha]

@beauby I agree that using exceptions for control flow isn't ideal. But what is worst is to rely on a race condition to validate data.

I've seen too many Rails apps with an uniqueness validation without the corresponding database constraint. I just want to avoid this situation in Hanami projects.

---

How to move on from this? I have a few suggestions:

1. I'm working on a spike to eventually include in Hanami 1.1, where the Hanami::Model::UniqueConstraintViolationError exposes the column/constraint that it has been violated via getters
2. With Introduce Hanami::Action::Params::Errors#add #232, you can add an error message, by inspecting the error. We're still in Hanami 1.1.
3. To have in Hanami 2.0, the errors to be returned instead of raised exceptions. A developer can match the returning value of a repository action and act accordingly. This is still an experiment.