Rails 4:如何在这个长控制器方法中解耦逻辑

Question

我正在将has_secure_password与rails 4.1.5应用程序一起使用。我想将我的登录功能与我的SessionsController解耦，这样我就可以重用它来登录我的应用程序中任何我想登录的用户，例如在注册后登录用户，记录分析事件等。

因此，我将代码重构为LoginUser服务对象，我对此感到满意。

问题是，在重构之后，我的控制器仍然有一些耦合的逻辑。我使用Form对象（通过reform gem）进行表单验证，然后将用户、会话和密码传递给LoginUser服务。

以下是我的SessionsController中的创建方法：

  def create
    login_form = Forms::LoginForm.new(User.new)
    if login_form.validate(params[:user]) # validate the form
      begin #find the user
        user = User.find_by!(email: params[:user][:email])
      rescue ActiveRecord::RecordNotFound => e
        flash.now.alert = 'invalid user credentials'
        render :new and return
      end
    else
      flash.now.alert = login_form.errors.full_messages
      render :new and return
    end
    user && login_service = LoginUser.new(user, session, params[:user][:password])
    login_service.on(:user_authenticated){ redirect_to root_url, success: "You have logged in" }
    login_service.execute
  end

一切都按预期进行，但我不满意的是验证表单和在将其发送到服务对象之前找到用户之间的逻辑。还有多个闪光警报的感觉。。好不对。

我该如何通过将这两者解耦来使这种方法变得更好？现在看来，一只背着另一只。

这里是我的LoginUser服务对象供您参考

class LoginUser
    include Wisper::Publisher
    attr_reader :user, :password
    attr_accessor :session
    def initialize(user, session, password)
        @user = user
        @session = session
        @password = password
    end
    def execute
        if user.authenticate(password)
            session[:user_id] = user.id
            publish(:user_authenticated, user)
        else
            publish(:user_login_failed)
        end
    end
end

Answer 1

这里最让我印象深刻的是，create是一个具有多个职责的方法，可以/应该隔离。

我看到的责任是：

1. 验证表单
2. 查找用户
3. 返回验证错误消息
4. 返回未知用户错误消息
5. 创建LoginService对象，在身份验证行为之后进行设置并执行身份验证

清理这一问题的设计目标是编写具有单一责任的方法，并在可能的情况下注入依赖项。

忽略UserService对象，我对重构的第一次尝试可能是这样的：

def create
  validate_form(user_params); return if performed?
  user = find_user_for_authentication(user_params); return if performed?
  login_service = LoginUser.new(user, session, user_params[:password])
  login_service.on(:user_authenticated){ redirect_to root_url, success: "You have logged in" }
  login_service.execute
end
private
def user_params
  params[:user]
end
def validate_form(attrs)
  login_form = Forms::LoginForm.new(User.new)
  unless login_form.validate(attrs)
    flash.now.alert = login_form.errors.full_messages
    render :new
  end
end
def find_user_for_authentication(attrs)
  if (user = User.find_by_email(attrs[:email]))
    user
  else
    flash.now.alert = 'invalid user credentials'
    render :new
  end
end

值得注意的是，return if performed?条件将检查是否调用了render或redirect_to方法。如果是，则调用return，并提前完成create操作，以防止出现双重渲染/重定向错误。

我认为这是一个很大的改进，因为责任已经被分成了几种不同的方法。在大多数情况下，这些方法都注入了它们的依赖项，这样它们在未来也可以继续自由发展。