代码之家 › 专栏 › 技术社区 › John Topley

轨道:保持用户欺骗检查干燥

refactoring ruby ruby-on-rails

1

John Topley · 技术社区 · 17 年前

出于一种非原始性,我使用RubyonRails编写了一个博客应用程序。我的 PostsController 包含一些代码,确保登录用户只能编辑或删除自己的日志。

我尝试将这段代码分解成一个私有方法,只需一个参数即可显示Flash消息,但当我这样做并通过编辑另一位作者的文章进行测试时,我得到了一个 ActionController::DoubleRenderError -“每个操作只能呈现或重定向一次”。

这些支票怎么保管 DRY ?显而易见的方法是使用before过滤器,但是 destroy 方法需要显示不同的闪光灯。

以下是相关的控制器代码:

before_filter :find_post_by_slug!, :only => [:edit, :show]

def edit

  # FIXME Refactor this into a separate method
  if @post.user != current_user
    flash[:notice] = "You cannot edit another authorâs posts."
    redirect_to root_path and return
  end
  ...
end

def update 
  @post = Post.find(params[:id])

  # FIXME Refactor this into a separate method
  if @post.user != current_user
    flash[:notice] = "You cannot edit another authorâs posts."
    redirect_to root_path and return
  end
  ...
end

def destroy
  @post = Post.find_by_slug(params[:slug])

  # FIXME Refactor this into a separate method
  if @post.user != current_user
    flash[:notice] = "You cannot delete another authorâs posts."
    redirect_to root_path and return
  end
  ...
end

private
def find_post_by_slug!
  slug = params[:slug]
  @post = Post.find_by_slug(slug) if slug
  raise ActiveRecord::RecordNotFound if @post.nil?
end

3 回复 | 直到 17 年前

1

2

Ian Terrell 17 年前

之前的过滤方法仍然是一个不错的选择。您可以使用控制器的 action_name 方法。

before_filter :check_authorization

...

protected

def check_authorization
  @post = Post.find_by_slug(params[:slug])
  if @post.user != current_user
    flash[:notice] = (action_name == "destroy") ? 
      "You cannot delete another authorâs posts." : 
      "You cannot edit another authorâs posts."
    redirect_to root_path and return false
  end
end

对不起,中间那个三元运算符。:)当然,你可以做任何你喜欢的逻辑。

如果愿意,也可以使用方法,如果方法失败,则通过显式返回来避免双重渲染。这里的关键是返回,这样就不会进行双重渲染。

def destroy
  @post = Post.find_by_slug(params[:slug])
  return unless authorized_to('delete')
  ...
end

protected

def authorized_to(mess_with)
  if @post.user != current_user
    flash[:notice] = "You cannot #{mess_with} another authorâs posts."
    redirect_to root_path and return false
  end
  return true
end

您可以通过将不同的行为部分(授权、处理糟糕的授权)拆分如下(在我看来),进一步简化它:

def destroy
  @post = Post.find_by_slug(params[:slug])
  punt("You cannot mess with another author's post") and return unless author_of(@post)
  ...
end

protected

def author_of(post)
  post.user == current_user
end

def punt(message)
  flash[:notice] = message
  redirect_to root_path
end

就我个人而言,我更喜欢将所有这些例行工作卸载到一个插件上。我个人最喜欢的授权插件是 Authorization . 在过去的几年里,我用它取得了巨大的成功。

这将重构控制器以使用以下变量:

permit "author of :post"

2

1

kajaco 17 年前

简单的答案是将信息改为既适合这两种情况的信息:“你不能干扰其他作者的帖子。”

3

1

mkomitee 17 年前

如果您不喜欢最后一个解决方案中难看的*返回,那么您可以使用around过滤器,并且只有在用户获得授权的情况下才有条件地生成。

around_filter :check_authorization, :only => [:destroy, :update]

private
def check_authorization
    @post = Post.find_by_slug(params[:slug])
    if @post.user == current_user
        yield
    else
        flash[:notice] = case action_name
        when "destroy"
            "You cannot delete another author's posts."
        when "update"
            "You cannot edit another author's posts."
        end
        redirect_to root_path
    end
end

*--这是我的偏好,尽管从代码上看它是完全有效的。我只是觉得这个款式不合适。

我还应该补充说,我还没有测试过这个,也不是百分之百确定它会起作用,尽管它应该很容易尝试。