Rails validating API parameters

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

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  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.

StackExchange Code Review Q#108708, answer score: 5