return from method used by before_filter

Question

I've inherited some rails code that checks to see if a user is defined in a method that is called by a before filter:

before_filter :get_user

def get_user
  @user = User.find(params[:id])
  if !@user
    return false
  end
end

Now, the problem is that this doesn't work :) If the user isn't found, we don't return from the controller, we just return from the get_user() method and then continue execution in show() or update() method with @user set to nil.

My simple solution is to add a redirect to get_user() if @user is nil:

def get_user
  @user = User.find(params[:id])
  if !@user
    redirect_back
    return false
  end
end

Now, my test are passing and everything seems right with the world. But ... I don't understand what's going on. Can someone please explain why the return in get_user() didn't stop execution in the controller completely but only breaks us out of get_user() and causes us to fall into the controller method that was originally called?

Thanks!

Answer 1

http://guides.rubyonrails.org/action_controller_overview.html#filters

"The method simply stores an error message in the flash and redirects to the login form if the user is not logged in. If a "before" filter renders or redirects, the action will not run. If there are additional filters scheduled to run after that filter, they are also cancelled."

Pretty self explanatory, but the nature is that you can't return to halt execution in a filter.

Answer 2

Return inside an method just returns and break that method and nothing else. Se code below.

def foo
  return "foo"
  return "bar"
end

puts my_method # this will puts "foo" and the second return will never be called.

However, in ruby you still can execute code after an return with ensure.

def bar
  return "bar"
ensure
  @bar = 'Hello world'
end

puts bar # returns and prints "bar" 
puts @bar # prints "Hello world" because the ensure part was still executed

And keep in mind that the last executed code in your method will be returned, so you don't always need to write return before your value. And if you have an ensure part in your method the last executed code before that will be returned if you haven't returned something already.

And theres no need to return false in your before filters. If i remember right rails before version 3.1 did stop the controller when an before filter was returning an falsy value. Nil is still falsy in ruby and to strip some rows you cud write your filter like below because if no user is found @user will be nil in this example.

def get_user
  @user = User.find_by id: params[:id] # I use find_by to prevent exception, else we may return an 500 error which you may or may not want.
  redirect_back unless @user
end

Answer 3

I would rewrite this code this way

def get_user
  redirect_back if User.where(id: params[:id]).empty?
end

for two reasons. First, why if and all that if you can check it in a more simple way. Second, find raises an exception if object is not found so this check makes no sense at all!