Given the following test:
shared_examples_for 'versioning' do
context 'with different versions for the same endpoint' do
context 'catch-all' do
before do
subject.version 'v1', macro_options
subject.get 'version' do
'v1'
end
subject.format :txt
subject.version 'v2', macro_options
subject.get 'version' do
'v2'
end
subject.route :any, '*path' do
error!('Not Found', 404)
end
end
it 'finds v1 of the endpoint' do
versioned_get '/version', 'v1', macro_options
expect(last_response.status).to eq(200)
expect(last_response.body).to eq('v1')
end
it 'finds v2 of the endpoint' do
versioned_get '/version', 'v2', macro_options
expect(last_response.status).to eq(200) # >>>>> This fails! Results in 404 <<<<<
expect(last_response.body).to eq('v2')
end
end
end
end
For even more context, see: https://github.com/ruby-grape/grape/compare/master...zherr:multiple-endpoint-catchall
Before digging deeper into a fix I would like to gather feedback from contributors on whether this appears to be a bug, or if this behavior intentional.
Happy holidays!
It certainly smells like doing version v1 then version v2 to the same api class overrides the previous definition, so split them up and mount them and that should work better.
This might be helpful.
Our current implementation splits up the versions by module like so, and still experiences the same issue:
module MyApp
class Base < Grape::API
mount V1::Base
mount V2::Base
route :any, '*path' do
error!('Not Found', 404)
end
end
end
Given this implementation, when a request specifies Accept-Version=v2 in the header, for example, the V2 endpoint is not found. When the version is left unspecified, the V1 endpoint is found just fine.
Unfortunately, if we switch the mount order here as the blog post suggests, we still run into the same issue if the catch-all is defined; then V1 endpoint is not found.
Removing the catch-all solves part of the issue, and allow requests to find the V1 endpoint when not specifying the version, and allows requests that do specify 'v2' to find the new version. But this leaves us unable to capture traffic that would typically 404 and let us render a reasonable message.
It still seems that the presence of the catch-all should not affect if an endpoint is available, mount order or otherwise. What do you think?
I can confirm that this is a bug. I PRed the spec with mount in https://github.com/ruby-grape/grape/pull/1722 (but it should work without mount too). I wasn't able to fix it after digging a bit, maybe someone will have better luck.
Most helpful comment
I can confirm that this is a bug. I PRed the spec with mount in https://github.com/ruby-grape/grape/pull/1722 (but it should work without mount too). I wasn't able to fix it after digging a bit, maybe someone will have better luck.