Inherit from Grape::API::Instance breaks Boolean type

[sondnm]

Recently I followed the UPGRADING.md guide to upgrade Grape from 1.0.3 to 1.2.4 and an error occurs due to the use of type: Boolean in params block.
This happens only when inheriting from Grape::API::Instance. I wrote a small rack app to test and the behaviour is the same.

# config.ru
require 'grape'

module App
  class API < Grape::API::Instance
    resource :system do
      params do
        optional :hit, type: Boolean
      end
      get :ping do
        "pong"
      end
    end
  end
end

run App::API

Error:

NameError: uninitialized constant App::API::Boolean

I could easily change the Boolean to Virtus::Attribute::Boolean to fix this issue on my app but since you already supported this Boolean type feature, I think it should be supported in the next releases too.

Thank you.

[dblock]

Feel free to PR a fix to this effect. I would also try to track where this got removed. There's probably a good reason since Boolean is not a native type in Ruby.

[myxoh]

Note: if you are just creating an API you should not be inheriting from ::Instance rather just from Grape::API

In the Upgrading.md guide the reference to when you should use Grape::API::Instance it is only when you are monkeypatching the class (which is not what you are doing here)

i.e. on this example you should be doing:

# config.ru
require 'grape'

module App
  class API < Grape::API
    resource :system do
      params do
        optional :hit, type: Boolean
      end
      get :ping do
        "pong"
      end
    end
  end
end

run App::API

[sondnm]

Thank you @myxoh for pointing that out. As I can see, the inheritance from Grape::API works great for a rack app.

However, on my app where I integrate grape with rails and using rspec together with rack-test, when I run the api specs, it returns 404 if it inherits from Grape::API. Notice that the APIs still works normally when running on a real server. Since I want to make sure the tests pass before deployment, I just let them inherit from Grape::API::Instance 😄 . That's where I encounter the Boolean problem.

FYI, my app's APIs structure would look similar to this.

Rails.application.routes.draw do
  mount API::Base, at: "/"
end

# app/controllers/api/base.rb

class API::Base < Grape::API
  mount API::V1::Base
end

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

class API::V1::Base < Grape::API
  version 'v1', using: :path
  format :json

  mount API::V1::Ping
end

# app/controllers/api/v1/ping.rb

class API::V1::Ping < Grape::API
  resource :system do
    params do
      optional :hit, type: Boolean
    end
    get :ping do
      "pong"
    end
  end
end

Edited: Add failing spec

# spec/requests/api/v1/ping.rb

require 'rails_helper'
require 'rack/test'

describe API::V1 do
  include Rack::Test::Methods

  def app
    API::V1::Ping
  end

  it 'returns pong' do
    get '/v1/system/ping'
    expect(last_response.status).to be(200)
  end
end

[myxoh]

Interesting as this is a similar setup to the one I've got. Could you please attach the failing spec too? I'll look into this!

Thanks !

[sondnm]

Oops! I should have attached it in the first place. I edited the above comment and added the failing spec.

Thanks!

[myxoh]

This is interesting, and sounds like a bug.
Just as a colour point, if you replace the

def app
   API::V1::Ping
 end

With

def app
  Rails.application
end

The spec will go green again.

[myxoh]

The problem stems from the fact that an undocumented breaking change from the upgrade is that Class.new(Grape::API) is no longer a proper Rack application, rather Class.new(Grape::API::Instance) is, this has very few real life implications but I'll see whether I can make it so that Class.new(Grape::API) is also a Rack app
Else, I'll add this to the breaking changes. Thanks for bringing this up 👍

[dblock]

I think we could use an integration test in Grape.

I upgraded ruby-grape/grape-on-rails#8 and it "just worked". This is because I don't have def app, that is not required for Rails, only to declare a Rack app in tests if you're using Rack::Test?

[myxoh]

OK, I've actually managed to replicate the issue on a spec in the rack integration, and also find a solution ( I believe)
I'll create a PR

[myxoh]

I think this issue can now be closed, could you confirm @sondnm whether you can now get your spec to work by inheriting from Grape::API ?

[sondnm]

It is working great now! Thanks a lot @myxoh.

Since the Boolean type for Grape::API::Instance is not relevant anymore, I will close this issue.