Why not refer to the session hash in the Model layer?

Question

This question has been repeatedly asked and answered with one-line assertions, such as "because it is an obvious violation of MVC." Frankly, I just don't get it. Indeed, it feels to me that putting session inside the controller merely an artifact that ApplicationController faces the network layer via a rack call, rather than an MVC mandate. Let me explain my problem.

Rolling an authentication from scratch, I found myself agonizing and spiking all over the place for lack of ability to articulate simple tests (session isn't available to the testing framework either). My authentication scheme, as almost all I have seen in rails, wanted to use the session hash as a persistence layer to retain the id for the User model of the "current user." Doesn't that feel FAR MORE like a model than a controller artifact?

The code smells are obvious whenever you look at the "typical" sessions controller (this one from Ryan Bates excellent screencasts). Desperate to shovel this concept in with rest, we see unhealthy language such as:

def create 
  user = User.find_by_email(params[:session][:email])
  if user && user.authenticate(params[:session][:password])
    session[:user_id] = user.id
    redirect_to root_url, notice: "Logged in!"
  else
    flash.now.alert = "Email or password is invalid"
    render "new"
  end
end

To me, this is a code smell, an obviously overlogicked controller that is screaming for refactoring! But we can't. Why? Oh yes, because it is a violation of MVC to put references to the session, used as a persistence lawyer into the model. WTF? Doesn't it say something to you that we seem to want to CALL THIS REST RESOURCE /sessions?

To see why this is just plain screwy, look at your login views -- hand-coded html, or use of the "_tags" API? If we had an ActiveModel model to do this code, then the create code could look like the usual scaffolding, or possibly even reduced to the "respond_with" one-liner.

def create 
  recognition = Recognition.new(params[:user])
  if recognition.save
    redirect_to root_url, :notice => "Thank you for signing up!"
  else
    render "new"
  end
end

Then, take a look at the hand-coded html for all of those views! If Recognition was a model, persisted by session (or some other means that should not be the responsibility of the controller layer anyway), then you could simple use form builder or simple_form to generate the forms. Of course, we could simply pass the session hash to a "new_login" class method of Recognition, say Recognition.on(session).new(params[:recognition]), but that seems uglier than it has to be. Perhaps it is inherent, as we will want to use a current_user reference later in the application layer, perhaps Recognition.on(session).current_user similar to the way one might use a singleton pattern?

Just try to build your authentication package with strict BDD, and honestly tell me you haven't spiked this portion of it? If we had a Recognition model, this entire thing would be reduced to a simple set of unit tests without hackery. Now, instead we have the "sole" use case for integration testing, magic invasions of ActiveController modules and hacks to make speedy any acceptance testing of the logged_in_as predicate.

I think the entire point of ActiveModel was to facilitate this kind of rethinking and refactoring. Not all models use "the" database. Why not persist to the "session?"

I have been using devise and its kin for too long, burying these smells with the "dont mess with the gem" excuse I don't have to look at them. No longer! I think I am going to reject the zealots from now on. Sorry, to me, session is a persistence layer that should be manipulated in the Model layer of MVC. I posit, but am not sure, that the reason it lives in controllerland has more to do with the ugly or elegant fact that controllers are rack objects than any theoretical MVC magic.

So once again, is there a more elegant way to access the session layer than to have logic in the controller for that?

Answer 1

Maybe it's just me, but I don't sniff the code smell in that controller. I guess it depends on what you think should go into controllers versus models.

I think people take the "skinny controllers" idea to an unhealthy extreme at times. Yes, you want everything that can be in a model to be on the model: but controllers exist to change models based on application state, and you should allow them to fulfill that design goal. Having every controller be something like:

def create
  Creator.create(params) # Pass all params to the creator model, which will automatically detect what object you're trying to create, make it, and then render the appropriate view
end

def show
  Shower.show(params) # Find the model object and the view based on the params and render them together
end

Defies the idea of separation of concerns and creates nightmarish problems for people trying to update and maintain your code. Controllers should call model objects and even methods on those models to create and persist application state, and models should be agnostic to the state of the application. If you couple them together too tightly you'll end up with view and controller code in your models, at which point it becomes very difficult to determine what's where in your application.

If this is what you want and you think it serves your goals, then go for it -- but your code will be more difficult to maintain and will be hard for other people to understand. MVC exists because it's a sensible way to separate concerns and makes your code less surprising for other people.

All of this said, the session model you're proposing is actually a pretty good idea... and hence, it already exists. ;) The authlogic framework has a Sessions model: when logging in using authlogic, you create a new UserSession with the params object. UserSessions live in the models folder and exist solely to abstract away the nitty gritty of authentication into a controller-aware model class, so if that's what you're looking for, it's already been done for you! Go check out the authlogic github repository for documentation and usage examples.

I would still shy away from passing any kind of controller state into a real ActiveRecord model though. Allow your controllers to manipulate models and render the results of that manipulation as HTML -- it's what they're there for!