Upgrading from 0.1.6 to 1.0.0

[summera]

I have a project using roar-rails 0.1.6 and everything is working great :ok_hand:

I want to upgrate to 1.0.0 but when I tried to do so I started receiving the following error in my specs:

Failure/Error: post v1_groups_path, json(:group), headers_for(:json)
     NoMethodError:
       undefined method `group_url' for #<Api::V1::GroupsController:0x007fcd83e7d5a8>
     # ./app/controllers/api/v1/groups_controller.rb:25:in `create'
     # ./spec/api/v1/groups_spec.rb:31:in `block (3 levels) in <top (required)>'

Everything except for the #create action is working fine. For some reason calling respond_with @group in #create throws the above error.

This is my GroupsController:

module Api
  module V1
    class GroupsController < ApplicationController
      include Roar::Rails::ControllerAdditions

      before_action :set_group, only: [:show, :update, :destroy]
      respond_to :json

      def index
        @groups = Group.paginate(page: params[:page], per_page: params[:per_page])

        respond_with @groups
      end

      def show
        respond_with @group
      end

      def create
        @group = consume! Group.new
        @group.save

        respond_with @group
      end

      def update
        consume! @group
        @group.save

        respond_with @group
      end

      def destroy
        @group.destroy

        head :no_content
      end

      private

      def set_group
        @group = Group.find params[:id]
      end
    end
  end
end

Any idea what's going on? Missing something simple maybe?

[summera]

Oh... and my GroupRepresenter:

module Api
  module V1
    class GroupRepresenter < BaseDecorator
      property :name
      property :video_playlists
      property :audio_playlists
      property :users

      link :self do
        v1_group_url represented
      end
    end
  end
end

module Api
  class BaseDecorator < Roar::Decorator
    include Roar::JSON
    include Roar::Hypermedia
  end
end

[apotonick]

I think group_url might be called because the responder wants to redirect to the created group. This helper is not available in the controller (apparently). Is group the correct name? It looks like it should be v1_group?

I am guessing the namespacing doesn't work, but that must be a responder problem. Did you also upgrade Rails or anything?

[summera]

Ahh ok. It's curious that all the other actions are fine but #create is throwing the error. Which name are you referring to?

v1_group is the correct helper.

My controllers and representers are in the Api::V1 namespace while my models are not namespaced. This is the route:

Rails.application.routes.draw do
  scope module: :api, defaults: { format: :json } do
    namespace :v1 do
      resources :groups
    end
  end
end

I kept rails and all the other gems the same when upgrading. I am using rails 4.1.5.

[apotonick]

That's probably because create is the only place where the responder computes a URL to redirect you.

It grabs the model and infers the URL path name, and since the model is not namespaced, it invokes group_path. I don't exactly know why that happens, but it must be something in the responder code. Can you find out whether the actual creation works, just to make sure it's not directly roar's fault?

[summera]

Yep! Just pried into the #create method and the creation of @group works and saves a group to the DB fine. It's only the respond_with @group call that seems to be causing the problem.

[summera]

So I did some digging and this is what I find out :satisfied:....

In version 0.1.6 roar-rails overrode the method #render_to_body of the AbstractController. This prevented Rails from calling it's own #render_to_body method, which eventually calls #process_options, which then calls #url_for(location) (where location is the group in this case) if a location exists. And this is where the error is occurring with v1.0.0 :boom:

Since v1.0.0 no longer overrides this method, it is reaching this point (because we no longer have this conditional) and trying to call #url_for(location). Here is an example of the options hash given to #process_options when creating a group:

{:status=>201,
 :location=>
  [#<Group id: 18, name: "Mr. Daren Block18", created_at: "2015-01-05 04:37:05", updated_at: "2015-01-05 04:37:05">],
 :prefixes=>["api/v1/groups", "application"],
 :template=>"create",
 :json=>
  #<Roar::Rails::Responder::Response:0x007fdcd5a0db38
   @body=
    "{\"name\":\"Mr. Daren Block18\",\"video_playlists\":[],\"audio_playlists\":[],\"users\":[],\"links\":[{\"rel\":\"self\",\"href\":\"http://127.0.0.1:3000/v1/groups/18\"}]}",
   @content_type=
    #<Mime::Type:0x007fdccc593570
     @string="application/json",
     @symbol=:json,
     @synonyms=["text/x-json", "application/jsonrequest"]>>,
 :layout=>
  #<Proc:0x007fdccd7d6070@/Users/ari/.rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/actionview-4.1.5/lib/action_view/layouts.rb:386>}

Execution never reaches any of the new methods, #_render_option_*, that were added to v1.0.0 before the exception is raised, as you can see in rails source here.

[apotonick]

Argh, when this change was introduced I knew something is gonna break. :facepunch:

Are you interested in resolving this?

[summera]

Yea definitely. I'll put in a PR :+1:

[summera]

@apotonick This doesn't seem to be an issue with roar-rails. The exception occurs because of the way I have defined my routes/namespacing and the way in which rails handles the Location header by default.

By default, Rails sets the location to be equal to the resources array which is what is passed to respond_with in the controller. In the example above, this is an array with an instance of Group, [#<Group id: 18, name: "Mr. Daren Block18", created_at: "2015-01-05 04:37:05", updated_at: "2015-01-05 04:37:05">]. It then calls url_for with this array. Since my url helper is defined as v1_groups this doesn't work and attempts to call group_url which is not defined.

However, overriding the location like this does work:

def create
  @group = consume! Group.new
  @group.save

  respond_with @group, location: [:v1, @group]
end

[apotonick]

Yeah, that's what I guessed. But how does Rails usually know that it has to prepend the controller's namespace v1_ to the url helper?

[summera]

Well by default RaiIs just passes along the resources passed to #respond_with. So, auto determining the location doesn't seem to be based on the controller being used at all. I could be wrong, but I think the only way Rails would know this automatically when using #respond_with in my case is if I were to namespace my models in the V1 namespace, which I don't want to do.

It would be nice if Rails looked-up the correct url helper in the routes for the created resource based on the current controller instance.

[apotonick]

But then let me ask you, how did it work before? Why did it call v1_group_url instead of group_url? Or are we both wrong and before the update it didn't call a URL helper at all (because doing something else, like sending back the representation document or so)?

Can you verify that the behaviour, other than the wrong URL helper, is identical to the old behaviour?

We can implement generic logic that considers namespaces in roar-rails or in the upcoming Endpoint layer in Trailblazer.

[summera]

Both wrong haha. Before the update it didn't call a URL helper at all. Because of the way roar-rails was overriding #render_to_body before the update, it wasn't setting the Location header and was not calling #url_for. The new behavior sets the content-type and the location headers.

Implementing some general logic that considers this sounds like a good idea to me :+1: . Are you thinking logic that is solely based on namespacing or something that is also based on a rails routes lookup?

[apotonick]

So the old behaviour was wrong?

I am thinking about something completely decoupled from Rails, so we can use it in Trailblazer and run that in Sinatra, Lotus, etc., too. Because that's the long-term goal of Trb.

[summera]

Yep, the old behavior was wrong and was fixed by https://github.com/apotonick/roar-rails/pull/74 . The only reason my code worked before was because of the incorrect behavior in 0.1.6.

[apotonick]

Great, so what the hell are we talking about then??? :laughing:

[summera]

Hahaha. Good question. We have confirmed that roar-rails #respond_with behavior is now correct! And also that when namespacing routes you often have to set the location manually since rails just passes the resource to #url_for.

[apotonick]

And this is what we could change in roar-rails and Trailblazer. A representer could have a configured namespace and use that knowledge.