Fix route discovery with api! for Rails 5 (passes tests) #526

aminariana · 2017-02-16T19:57:10Z

I am building this PR on top of PR #465 from June 8, 2016. That one never made it to master because of bad Gemfile values that later got fixed in master, but never got merged into the PR.

In this PR:

I fix the issue with api! route discovery by borrowing @sblackstone fix commit
I add test coverage against Ruby 2.3.3
I fix the failing tests by merging the latest Gemfile fixes from master into this PR

Travis is passing 100% for me locally.

Fixes Issue #415
Fixes Issue #495
Fixes Issue #510
Distant relationship with Issue #522
Related to PR #465

Since rails deprecated some behavior that the old code depended on, we just change it to match what its been doing under the hood all along and avoid having to do a depth first search at the same time.

…ions

…eck your routes. (ArgumentError)

iNecas · 2017-02-16T23:18:13Z

Hi, I think I have a version that has actually tests passing with rails 5, will push in few minutes

iNecas · 2017-02-16T23:30:22Z

#527, finger crossed

aminariana · 2017-02-17T01:23:20Z

@iNecas My latest changes make it fully compatible with Rails 5.0 and Ruby 2.2.2+. However, I was forced to make it a breaking change, losing Rails <4 and Ruby <2.2. If yours is more compatible, great, if not please consider accepting this one.

aminariana · 2017-02-17T01:24:50Z

@iNecas by the way would be awesome to fix the Rails 5 deprecation warnings.

iNecas · 2017-02-17T07:39:45Z

I managed to keep it compatible in #527 (needed to drop ralis 32, but it was about a time), the deprecation warnings will be addressed separatly, such as here #420

iNecas · 2017-02-17T13:14:18Z

Closed by #527, thanks @aminariana, you helped a lot speeding up the process!

jsantos · 2017-02-27T08:39:12Z

Any hint why after this commit namespaced controllers would stop working?

I got the following error with this commit:

/.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:268:in `const_get': uninitialized constant SurveyorUsersController (NameError)
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:268:in `block in constantize'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:266:in `each'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:266:in `inject'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/inflector/methods.rb:266:in `constantize'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.1/lib/active_support/core_ext/string/inflections.rb:66:in `constantize'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/application.rb:58:in `route_app_controller'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/application.rb:64:in `block in routes_for_action'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/application.rb:63:in `select'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/application.rb:63:in `routes_for_action'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/apipie_module.rb:18:in `method_missing'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/method_description.rb:171:in `api_data'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/method_description.rb:26:in `initialize'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/application.rb:87:in `new'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/application.rb:87:in `block in define_method_description'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/application.rb:79:in `each'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/application.rb:79:in `define_method_description'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/apipie_module.rb:18:in `method_missing'
  from /.../.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/apipie-rails-d3519c357caf/lib/apipie/dsl_definition.rb:408:in `method_added'
  from /.../my_rails_app/app/controllers/api/sms/surveyor_users_controller.rb:46:in `<class:SurveyorUsersController>'
  from /.../my_rails_app/app/controllers/api/sms/surveyor_users_controller.rb:1:in `<top (required)>'

It claims not to be able to find SurveyorUsersController, that in fact is Api::Sms::SurveyorUsersController.

iNecas · 2017-02-27T09:14:32Z

@jsantos seems like this code might be responsible for it https://github.com/Apipie/apipie-rails/pull/526/files#diff-c252ba104efd0ecb906ad2edd58ce884R58 ?

jsantos · 2017-02-27T10:10:51Z

@iNecas That's my best guess.

I'm just wondering if there's anything wrong on my end, but before this commit (and on Rails 4.2.x) everything was running smooth. I was on the middle of the transition to Rails 5 and after upgrading apipie-rails to 0.4.0 I bumped on this.

iNecas · 2017-02-27T10:54:12Z

@jsantos I will not get sooner to sending a fix for this until middle of this week: if you want to give it a try and fix it sooner, I will be more than happy to review.

jsantos · 2017-02-27T12:23:28Z

@iNecas Sure, I'll give it a shot 👍

jsantos · 2017-02-28T04:08:40Z

@iNecas Looks like this was a problem on our end (there was a route with no controller associated).

Shall we display a more informative error message when this happens?

iNecas · 2017-02-28T07:48:34Z

👍 on that. Will you give it a try: it would be great, as you know what the reproducer was

kklw · 2017-03-30T08:03:20Z

Hi, on adding api!, I'm getting NameError - uninitialized constant Api::V1::JobCreationsController: . Did i miss out something?

jsantos · 2017-03-30T08:05:36Z

@kklw Check if you have unused routes in your code. Doesn't need to be exactly the one that showed up in your stacktrace, but that was my case.

kklw · 2017-03-30T08:28:03Z

@jsantos Thanks! Works perfectly now.

jsantos · 2017-03-30T11:57:19Z

@iNecas Regarding this "issue" with non-existent routes: What's the best approach? Shall we present a clearer error message?

Because continuing silently here doesn't look like a good practice, I'd say.

Stephen Blackstone and others added 11 commits June 8, 2016 11:20

Fix route discovery with api! for Rails 5

7454140

Since rails deprecated some behavior that the old code depended on, we just change it to match what its been doing under the hood all along and avoid having to do a depth first search at the same time.

Test against the newest ruby versions, 2.3.3 and 2.4.0

1a2fc75

Ruby 2.4.0 did not work due to casting problems. Revert.

80108df

Fix Gemfile for Rails 4.2

5be29f3

Catch up Gemfile changes to the latest on Apipie master

97d8a93

Merge branch 'master' into master

391d3ee

Try testing against Rails 5.0

b723a6f

Configure for Rails 5. Exclude incompatible Ruby and Rails 5 combinat…

ba1d58c

…ions

Fix Rails 5 bug - Missing :action key on routes definition, please ch…

b51782d

…eck your routes. (ArgumentError)

Fix - block in constantize: uninitialized constant

0bfd992

Get rid of the redundant namespace

4fd5336

iNecas mentioned this pull request Feb 16, 2017

Hopefully the final fix for Rails 5 compatibility #527

Merged

aminariana added 3 commits February 16, 2017 16:56

Rails 5.0 compatibility

9ffe23d

Breaking change with Ruby <2.2 and Rails <5.0

2044da9

Ruby 2.2.2+ compatibility

c19b275

iNecas closed this Feb 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix route discovery with api! for Rails 5 (passes tests) #526

Fix route discovery with api! for Rails 5 (passes tests) #526

aminariana commented Feb 16, 2017

iNecas commented Feb 16, 2017

iNecas commented Feb 16, 2017

aminariana commented Feb 17, 2017

aminariana commented Feb 17, 2017

iNecas commented Feb 17, 2017

iNecas commented Feb 17, 2017

jsantos commented Feb 27, 2017

iNecas commented Feb 27, 2017

jsantos commented Feb 27, 2017 •

edited

Loading

iNecas commented Feb 27, 2017

jsantos commented Feb 27, 2017

jsantos commented Feb 28, 2017 •

edited

Loading

iNecas commented Feb 28, 2017

kklw commented Mar 30, 2017

jsantos commented Mar 30, 2017

kklw commented Mar 30, 2017

jsantos commented Mar 30, 2017

Fix route discovery with api! for Rails 5 (passes tests) #526

Fix route discovery with api! for Rails 5 (passes tests) #526

Conversation

aminariana commented Feb 16, 2017

iNecas commented Feb 16, 2017

iNecas commented Feb 16, 2017

aminariana commented Feb 17, 2017

aminariana commented Feb 17, 2017

iNecas commented Feb 17, 2017

iNecas commented Feb 17, 2017

jsantos commented Feb 27, 2017

iNecas commented Feb 27, 2017

jsantos commented Feb 27, 2017 • edited Loading

iNecas commented Feb 27, 2017

jsantos commented Feb 27, 2017

jsantos commented Feb 28, 2017 • edited Loading

iNecas commented Feb 28, 2017

kklw commented Mar 30, 2017

jsantos commented Mar 30, 2017

kklw commented Mar 30, 2017

jsantos commented Mar 30, 2017

jsantos commented Feb 27, 2017 •

edited

Loading

jsantos commented Feb 28, 2017 •

edited

Loading