Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Nested Namespaces worked in 1.4.8, broken in 2.0.0 #1310

Closed
aren55555 opened this issue Jun 9, 2017 · 7 comments
Closed

Nested Namespaces worked in 1.4.8, broken in 2.0.0 #1310

aren55555 opened this issue Jun 9, 2017 · 7 comments
Labels
debug regression sinatra-contrib
Milestone
v2.0.1

Comments

@aren55555
Copy link

aren55555 commented Jun 9, 2017 •
edited
Loading

I'm not sure if nested namespaces are actually supported as I don't see any specific mention of them in the docs. Anyhow in Sinatra 1.4.8 I could define an app like:

require 'sinatra'
require 'sinatra/contrib'
require 'json'

BOOKS = {
  1 => {name: 'The Lord of the Rings'},
  2 => {name: 'The Chronicles of Narnia'},
  3 => {name: 'Harry Potter'},
}

namespace '/api' do
  namespace '/v1' do
    namespace '/books' do
      get do
        body JSON.dump(BOOKS)
      end
    end
  end
end

cURLing the endpoint:

$ curl localhost:4567/api/v1/books
{"1":{"name":"The Lord of the Rings"},"2":{"name":"The Chronicles of Narnia"},"3":{"name":"Harry Potter"}}

Now if I define a similar app in Sinatra 2.0.0:

require 'sinatra'
require 'sinatra/contrib'
require 'json'

BOOKS = {
  1 => {name: 'The Lord of the Rings'},
  2 => {name: 'The Chronicles of Narnia'},
  3 => {name: 'Harry Potter'},
}

namespace '/api' do
  namespace '/v1' do
    namespace '/books' do
      get do
        body JSON.dump(BOOKS)
      end
    end
  end
end

And then cURL the same path:

$  curl localhost:4567/api/v1/books
<!DOCTYPE html>
<html>
<head>
  <style type="text/css">
  body { text-align:center;font-family:helvetica,arial;font-size:22px;
    color:#888;margin:20px}
  #c {margin:0 auto;width:500px;text-align:left}
  </style>
</head>
<body>
  <h2>Sinatra doesn’t know this ditty.</h2>
  <img src='http://localhost:4567/__sinatra__/404.png'>
  <div id="c">
    Try this:
    <pre>get &#x27;&#x2F;api&#x2F;v1&#x2F;books&#x27; do
  &quot;Hello World&quot;
end
</pre>
  </div>
</body>
</html>

With this 2.0.0 Sintra app I decided to list all the GET paths:

Sinatra::Application.routes["GET"].each do |route|
  puts route[0]
end

Which results in:

/api/v1/v1/books

If I cURL that path I get a successful result:

$  curl localhost:4567/api/v1/v1/books
{"1":{"name":"The Lord of the Rings"},"2":{"name":"The Chronicles of Narnia"},"3":{"name":"Harry Potter"}}

The repeated v1/v1 is not expected. Sinatra 1.4.8's /api/v1/books is the behaviour I would have expected.

Is this supposed to work? If there is a better way to define nested namespaces please let me know.
@mencargo
Copy link

We are experiencing the same problem.
Sinatra 2 introduced Mustermann as a dependency for string matching, the errors are originating from that gem.

An weird behavior is that if the second level namespace is a param like /:version instead of /v1 in the example above, you would get:

lib/mustermann/ast/validation.rb:34:in `check_name': can't use the same capture name twice: "/{version}/{version}/books/?" (Mustermann::CompileError)

@aren55555
Copy link
Author

@mencargo I've seen that same error too when trying to setup params within namespace; however when I reported the issue I was trying to make it as simple as possible. Both scenarios used to work and now no longer do.

However, I don't see any documentation indicating that they were ever supported and I am just hoping we get some clarification on this issue. It's blocking my upgrade from 1.x to 2.x.

@zzak
Copy link
Member

zzak commented Jul 4, 2017

I wonder if this is similar to #1251 or affected in some way.

There is also some coverage for nested namespaces here:

sinatra/sinatra-contrib/spec/namespace_spec.rb

Lines 468 to 478 in 69a7b8f

describe 'nesting' do
it 'routes to nested namespaces' do
namespace '/foo' do
namespace '/bar' do
send(verb, '/baz') { 'OKAY!!11!'}
end
end
expect(send(verb, '/foo/bar/baz')).to be_ok
expect(body).to eq('OKAY!!11!') unless verb == :head
end

@zzak
Copy link
Member

zzak commented Jul 4, 2017

/cc @mwpastore @namusyaka

@namusyaka
Copy link
Member

I'm going to take a look at this issue in a few days.

@zzak
Copy link
Member

zzak commented Jul 4, 2017

@namusyaka お願い致します！ 🙏

@namusyaka namusyaka mentioned this issue Jul 4, 2017
Merged
@namusyaka
Copy link
Member

@aren55555 @mencargo @zzak Could you confirm my pull request(#1322)?

@olleolleolle olleolleolle mentioned this issue Sep 5, 2017
Closed
@foxxx0 foxxx0 mentioned this issue Oct 12, 2017
Closed
@namusyaka namusyaka added this to the v2.0.1 milestone Feb 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
debug regression sinatra-contrib
Projects
None yet
Milestone
v2.0.1
Development

No branches or pull requests
4 participants
@zzak @aren55555 @namusyaka @mencargo