Rails validating API parameters

Question

I wanted to keep my API request logic separate form the controller logic. I therefore make use of a separate model (EmailChecker) that creates an instance of this class via EmailChecker.new(params).

Then I can check the new instance with valid?.

When it returns false I grep the errors and return it. This example only checks email but of course you can use any Rails validations on multiple parameters.

Problem is that I have to create a class for every API request you want to validate. Actually I don't mind because it makes the code easy to understand but it's not very DRY.

Any suggestions to improve this code?

class EmailChecker include ActiveModel::Validations include ActiveModel::Conversion include ActiveModel::Naming attr_accessor :email validates_format_of :email, :with => /\A[-a-z0-9_+\.]+\@([-a-z0-9]+\.)+[a-z0-9]{2,4}\z/i # validates :email, presence: true def initialize(attributes = {}) attributes.each do |name, value| send("#{name}=",value) end end def persisted? false end end #emailchecker 

Answer 1

You've essentially created a "View Model" for your API requests, which there's nothing wrong with doing that. Sometimes you want to abstract away your internal Domain Model and not have it exposed as params in the request.

That being said, you can DRY things up a bit by creating a base class:

class ApiModel include ActiveModel::Validations include ActiveModel::Conversion include ActiveModel::Naming EMAIL_FORMAT = /\A[-a-z0-9_+\.]+\@([-a-z0-9]+\.)+[a-z0-9]{2,4}\z/i def initialize(attributes = {}) attributes.each do |name, value| raise KeyError, "Attribute key '#{name}' is not supported" unless respond_to? "#{name}=" send "#{name}=", value end end def persisted? false end end 

Now your EmailChecker class becomes a 4-liner:

class EmailChecker < ApiModel attr_accessor :email validates_format_of :email, :with => EMAIL_FORMAT end 

Next, I'd like to focus on the initialize method. In your version:

def initialize(attributes = {}) attributes.each do |name, value| send("#{name}=",value) end end 

The send method call is left unguarded. You can specify keys in the Hash that do not correspond to setter methods in your object, leaving developers scratching their heads about why a method is not supported. Instead, this is an opportunity to fail early, and fail loud[er]. I would test to see if the attribute key is a valid setter, and then throw your own exception:

def initialize(attributes = {}) attributes.each do |name, value| raise KeyError, "Attribute key '#{name}' is not supported" unless respond_to? "#{name}=" send "#{name}=", value end end 

Raising a KeyError is more appropriate here, because the real problem is having an incorrect key in the attributes argument. The error message is informative to developers so they can fix the problem, yet does not give away any secrets.

For instance, if you send { foo: 'bar' } as the attributes, then you'll get the following error message:

KeyError: Attribute key 'foo' is not supported

The persisted? method is kind of confusing to me. This object is not persisted, so why even have this method? Unless this is overriding functionality baked in from the ActiveModel mixins, I would just remove it.

Answer 2

If you have different validation rules for different API calls, then I see no problem to extract them into different classes. A side note: DRYing out code is not free, it introduces new abstraction that results in a new layer of indirection, which doesn't help much to see the details of what's happening.

I'm not sure if you need to include ActiveModel::Conversion and ActiveModel::Naming. At least in Rails 5 including only ActiveModel::Validations is enough.

You can override the read_attribute_for_validation method instead of doing metaprogramming in the constructor.

I'd use a base class to let the validation rule classes to concentrate on the actual rules.

How about something like this?

module ApiParamsValidation class Base include ActiveModel::Validations attr_reader :params def initialize(params) @params = params end def read_attribute_for_validation(key) params[key] end end end module ApiParamsValidation class Email < Base EMAIL_VALIDATION_PATTERN = /\A[-a-z0-9_+\.]+\@([-a-z0-9]+\.)+[a-z0-9]{2,4}\z/i validates :email, format: { with: EMAIL_VALIDATION_PATTERN } end end 

You can read more about a nested case with some extra tests here.