grape icon indicating copy to clipboard operation
grape copied to clipboard

Grape::Exceptions::UnknownValidator

Open gottfrois opened this issue 10 years ago • 14 comments

I have the following files:

# app/controllers/api/v2/base.rb
module Api
  module V2
    class Base < Grape::API
      mount V2::Jobs
    end
  end
end

# app/controllers/api/v2/jobs.rb
module Api
  module V2
    class Jobs < Grape::API
      resources :jobs do
        params do
          requires :id, type: String, uuid: true
        end
        get ':id' do
        end
      end
    end
  end
end

# app/controllers/api/v2/validators/uuid.rb
module Api
  module V2
    module Validators
      class Uuid < ::Grape::Validations::Base

        REGEX = %r{[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}}

        def validate_param!(attr_name, params)
          unless (params[attr_name] =~ REGEX).zero?
            fail ::Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: 'must be a valid UUID'
          end
        end

      end
    end
  end
end

If I want the above to work, I have to manually require the validator class in base.rb file, like so:

require_relative './validators/uuid'

which result in throwing warnings:

app/controllers/api/v2/validators/uuid.rb:6: warning: already initialized constant Api::V2::Validators::Uuid::REGEX
app/controllers/api/v2/validators/uuid.rb:6: warning: previous definition of REGEX was here

since rails will automatically require the validator as well. Any idea why grape is not able to pick up the custom validator? Looks like a require order issue or something. Any clue?

Thanks

gottfrois avatar Oct 07 '15 08:10 gottfrois

Yes, It seems like a require order issue, how are you loading grape in rails?

towanda avatar Oct 07 '15 13:10 towanda

nothing fancy, i have grape in the gemfile and have this in my route:

mount Api::V2::Base => '/'

gottfrois avatar Oct 07 '15 13:10 gottfrois

Any idea why grape is not able to pick up the custom validator?

grape doesn't have an autoloading mechanism for custom validators (PR are welcomed).

In your case you can use require_dependency

# app/controllers/api/v2/base.rb
require_dependency 'api/v2/validators/uuid'

But I should explain why this happens. First, how does grape load buit-in validators?

  • Validator files are explicitly required in grape.rb
  • After the code was loaded and processed, inherited callback is invoked and starts the "registration" process.
  • All registered validators are saved in Grape::Validations.validators hash, where the keys are validator short names (ex. :uiid). When params is processed and the short name is not found in this hash then an UnknownValidator exception is raised.

In short, grape validators are "auto-registered" only if the validator file is explicity loaded.

Second, Rails autoloading relies on #const_missing to load constants. Obviously, the constant will be autoloaded only if we have its reference in the code.

In your case: Api::V2::Base const is specified in routes.rb => loads base.rb V2::Jobs in base.rb => loads jobs.rb Api::V2::Validators::Uuid is not specified in jobs.rb, only the short name and this is why rails doesn't load your custom validator.

P.S. as you can see from the explanation some :scream: things can be done:

# app/controllers/api/v2/base.rb

Api::V2::Validators::Uuid # instead of require_dependency 'api/v2/validators/uuid'

module Api
  module V2

dm1try avatar Oct 08 '15 12:10 dm1try

Thanks for the great explanation, makes perfect sense now. I'm guessing we could improve things on grape by looking up for the constant when we hit the :uuid value in validator parameters?

gottfrois avatar Oct 08 '15 12:10 gottfrois

@gottfrois this sounds reasonable, but I think we should register all available validators(bullt-in/custom) before a hitting anything(validators_paths helper would be helpfull to register a bunch of validators from different directories).

Also I'm personally don't like the implementation using inherited callback(the validator code must be eager loaded for this).

ok, the 3th solution :)

    class Base < Grape::API
      Grape::Validations.register_validator("uuid", Api::V2::Validators::Uuid) # by design it will be called twice
    end   

@dblock any thoughts?)

dm1try avatar Oct 08 '15 17:10 dm1try

Since grape params are declarative, we could simply add a constant lookup strategy in place that will be trigger when the ruby file is loaded. Am I missing anything? We don't have to look for the constant when someone hits the endpoint.

Also I'm personally don't like the implementation using inherited callback(the validator code must be eager loaded for this).

Yeah sounds like an easy solution. Ideally you would look/require only used constants.

gottfrois avatar Oct 08 '15 22:10 gottfrois

Since grape params are declarative, we could simply add a constant lookup strategy in place that will be trigger when the ruby file is loaded. Am I missing anything?

You should be able to resolve the constant nesting in that place (from :uuid to Api::V2::Validators::Uuid).

dm1try avatar Oct 09 '15 01:10 dm1try

@gottfrois take a look on the referenced issue, I've implemented some part

dm1try avatar Oct 16 '15 19:10 dm1try

What is the status of this? Has there been any progress on this validator issue (specifically with :uuid) in the last 2 years?

kevinelliott avatar Apr 15 '17 04:04 kevinelliott

@kevinelliott Doesn't look like it, feel free to PR.

dblock avatar Apr 15 '17 14:04 dblock

😢 Run into this bug(?) using grape in rails. Still not fixed.

koushinrin avatar Jul 06 '17 03:07 koushinrin

I have an issue related to this. I had one private api where I loaded all my api validators at the root of the grape api file:

Dir[File.dirname(__FILE__) + '/validators/*.rb'].each {|file| require file }

Now I have to create a second public api on the same rails app. I did the same to load all my public api validators on the root of the public api file. The issue is that some validators have the same name and conflicting, for example category_id_validation below will raise either a MyApi::V1::Exceptions or a MyApiPublic::V1::Exceptions ex:

class CategoryIdValidation < Grape::Validations::Base
  include MyApi::V1::Helpers::Authentication

  def validate(request)
    if request.params[:category_id].present?
      unless current_user(request).categories.map(&:id).include?(request.params[:category_id])
        raise MyApi::V1::Exceptions::ParamsIdNotExist, "category #{request.params[:category_id]} does not exist"
      end
    end
  end
end

It would be good to be able to load manually validators for each API so that we can have scoped validators and don't have name conflicts.

The only solution I can think of for now is having to rename validators on the second api like public_api_category_id_validation to not conflict with first validators from the other api, which is not great...

loicginoux avatar Oct 23 '19 10:10 loicginoux

It would be good to be able to load manually validators for each API so that we can have scoped validators and don't have name conflicts.

agreed

dblock avatar Oct 23 '19 14:10 dblock

Has anyone figured out how to use this with Rails 6/zeitwerk?

aesyondu avatar Sep 15 '21 02:09 aesyondu