Fat Models? Fat Controllers? Services to the rescue!!!

Sat, 07 Nov 2015 15:55:00 +0100

MVC has become the “de facto” standard for building web (and not only web) application. Enforcing the separation of concerns between presentation and business logic, MVC has become a sort of “common land” that makes easy to follow the application flow in a cross language manner.

Models, Controllers and Business …

Why are we talking about Fat Controllers and Models?

Mvc leaves to developers the freedom to choose where to place and elaborate the business logic of the application. When we talk about business logic we refer to a set of classe (library) that actually encapsulate the actions/methods that represent the application identity. If for example we are coding an e-commerce app we will need logic about what products I sell, how and where do I configure these products etc… All this logic should be the core app library that exposes an API to be used through an MVC pattern. When all or part of this logic is coded into controllers and/or models we talk about fat controllers and models.

Why is this an issue? Having fat controllers and models says that our business logic is sparsed all around and that when the application grows will be more and more difficult to maintain and refactor our codebase.

At the end we are talking about coding a flexible codebase respecting some well known rule like the single responsibility principle. Something that is fat in terms of methods and dependencies will for sure brake this basic rule messing things around.

Before starting with the code let’s make something clear about models and controllers.

a MODEL represents the instance of a defined entity that makes part of the application domain. Any method called on the model instance should be oriented to expose some aspect of the instance itself.
a CONTROLLER is an endpoint. It should only be concerned about receiving a call, giving the application business logic a place to execute what needs to be executed and render the view using the provided data.

Example. A Rails basic app.

Let’s say we are coding an application that has a model called Task with some basic attributes:

 # Task.rb
 class Task
    include Mongoid::Document
    field :name
    field :description
 end

If we want to persists a task we normally implement the logic into a method in a TasksController:

 #tasks_controller.rb
 class TaskController < ApllicationController
 
    def create 
        task = Task.new params[:task]
        if task.save
         ........
    end
 
 end

This is quite basic and works well even if, from a theorical point of view, adding an ORM as mongoid in our example extends the model responsibility over what he should know about the world around. But since ORM makes our life much more easy and productive we are happy to accept this as a minor side effects.

Now let’s go a bit further. A new story in the next sprint says that once a Task has been created we must enqueue a background job to notify some other app and even send a notification to every user that joined our account. We think this is easy enough and we add this logic to our controller:

#tasks_controller.rb
class TaskController < ApplicationController 
    def create 
        @task = Task.new params[:task]
        if @task.save
            _enqueue_job
            _notify_taks_to_users
         ........
    end
    
    def _enqueue_job
       .....	
    end
    
    def _notify_taks_to_users
        .....
    end

 end

This 2 new methods should already “smell” cause we are adding business logic to the controller that should not be aware of what happens when a Task is created. Our controller is getting FAT. But, at the end, this was easy and we closed the story very fast!! Is friday and a cold beer is waiting for us.

The next sprint complicates a bit our life. We are asked to add a module to the admin area of our application where the users with SUPERADMIN role should be able to manage tasks as well in any application account scope.

We add an /admin/tasks_controller to our application cause we cannot reuse the other controller cause they are base on different acl rules, etc….

#/admin/tasks_controller.rb
class TaskController < ApllicationController

    def create 
        @task = Task.new params[:task]
        if @task.save
            _enqueue_job
            _notify_taks_to_users
         ........
    end
    
    def _enqueue_job
       .....	
    end
    
    def _notify_taks_to_users
        .....
    end
end

Here we have an issue. We are duplicating the same business logic in 2 different points. Even if we are lazy coders we see that this is not a safe way. We could create a module to be included but more than a solutions looks more like hiding the problem.

Looking at the code we see that the 2 extra methods we added belongs to a particular phase of our application. Any time a task is created we must enqueue a job and send out a notification. So maybe this duties belongs to the Task model itself. Great! We think we are done. We reset the 2 controllers to the basic state:

#tasks_controller.rb
#/admin/tasks_controller.rb_
class TaskController < ApllicationController

    def create 
        task = Task.new params[:task]
        if task.save
         ........
    end

end

and we rely on the Task model callback to accomplish our post creation duties:

# Task.rb
class Task
    include Mongoid::Document
    field :name
    field :description
    
    after_create :_enqueue_job, :_notify_taks_to_users
    
    def _enqueue_job
       .....	
    end
    
    def _notify_taks_to_users
        .....
    end	
end

After running our test suite ee already note a downside. Our code works but now anytime a Task instance is persisted the 2 callbacks gets fired. Even if we create the instance from a spec factory the callbacks are fired …. We do not like this because it adds unwanted noise to any Task creation but we think we will survive. The controllers are skinny, the model is getting FAT … but we decide we are still going well.

And then comes the pain! We are asked to send a different notification to the user that created the task (actually the current_user) but not if the user is the SUPERADMIN.

How we can do that? The business logic is now completely managed by the Model that does not know nothing about the concept of Task creator. We could add this info as a field to the Model but is not something that is not required to be persisted.

Btw there is always a solution…..

 # Task.rb
 class Task
    ....
    attr_accessor :creator
    after_create :_enqueue_job, :_notify_taks_to_users, :_notify_creator
    .....

    def _notify_taks_to_users
        notify anyone
    end

    def _notify_creator
        notify @creator unless @creator.superadmin?
    end

 end
  # in both the tasks controllers
 def create 
    task = Task.new params[:task]
    task.creator = current_user
    if task.save
     ........
 end

We can do this so why is this bad?

we are adding a not required external dependencies to a Model. From now any time a Task instance is persisted the creator accessor must be provided. Over that the Task model also needs to be aware of the User role ….
we complicate our test suite factory. We must provide a creators of differents roles.
we open a way to add more and more dependencies and we will end with a model that plays more and more roles.

Services to the rescue

We could easily solve the problem encapsulating the business logic that stays behind the creation of a Task into his own class. A class that makes part of the application library and is responsible just for creating tasks. We reset the model to the basic implementation. Then we add a service class:

  #/app/services/create_task_service.rb
  class CreateTaskService 
    attr_accessor :current_user, :task_params
    
    def initialize(args={})
      @current_user = args.fetch(:current_user, params) do 
        raise ArgumentException 'Current user is missing!'
      end	
      @task_params = args.fetch(:params, {})
    end
    
    def run! 
      task = Task.new task_params
      _after_create(task) if @task.save!
      task
    end
    
    def _after_create(task)
      _enqueue_job(task)
      _notify_taks_to_users(task)
    end
    
    def _enqueue_job(task)
       .....	
    end

    def _notify_taks_to_users(task)
        notify anyone
        notify current_user unless current_user.superadmin?
    end
    
  end

What about our 2 controllers?

 #tasks_controller.rb
 #/admin/tasks_controller.rb_
 class TaskController < ApplicationController

    def create 
        begin
            @task = CreateServiceTask.new(
                current_user: current_user, params: params[:task] 
            ).run!
        
        rescue => e
          #handle 
        end
    end

 end

What are the benefits here?

CreateTaskService is a brick of our app core library. It just does one thing, creates a task, exposing a clear API. It requires a user, the task params and returns a task instance. Being devoted to this particular task is completely acceptable that it has dependencies like a Background job engine or a MailService.
Testing a plain ruby class is easy and fast.
Testing the business logic in completely isolation from the framework.
Approaching new feature is easier because we can really focus on the logic coding and then use it inside of the framework. A similar approach to coding a rest api.
We slowly build the core application that exposes all the methods we need. When we add new features we will look into this API as when we look for endpoints in a REST api. More services will means more endpoint to rely on.
We do not mess things up in our spec factories. A task_factory will give us back a Task instance following our attributes configuration. No callbacks. No tricks.

Does implementing a Service layer slow development down?

In my experience no. Once the pattern is clear and respected by any team member it can also increase the team productivity. Is much more easy to code on expectations when you know that your teammate is developing a class that will require some params and will give you back what you expect. You do not need to wait the real code as you do with rest API. Over that, when the application grows, you will have a set of self documented class that can help anybody to better understand the application logic and find the right method to use.

Notes:

using the name services with a run method is just a convention. At the end we are talking about plain classes. The only important thing to keep in mind is that a class is done to perform a task and that you must be able to rely on the methods signature and returns.

Happy coding!!!

Fake xhr request from rspec

Thu, 28 Feb 2013 16:36:00 +0100

I discovered today that I had no idea of how to fake an xhr request in an Rspec controller test. I need this cause some of my layout strategy behaves on the request.xhr? method. As expected Rspec solved the issue in a cool and elegant way.

	# normal request
	get :show, id: 1  
	# exactly the same request, This time fake to be an xhr call. request.xhr? will be true
	xhr :get, :show, id: 1

ruby alias vs alias_method

Thu, 29 Nov 2012 00:00:00 +0100

While alias and alias_method looks very similar at first they hide a substantial different behaviour. Look at the following code.

### Basic alias behaviour

class Boss
  def name
     p "Andrea"
  end
  alias :full_name :name
end

Boss.new.name # => Andrea
Boss.new.full_name # => Andrea

This is the the basic behaviour we could expect from the alias method. #full_name is perfectly aliased to #name. But what happens if we subclass Boss?

### Alias in subclass

class Employee < Boss

    def name
       p "Bob"
    end

end

Employee.new.name # => Bob
Employee.new.full_name  # => Andrea

Looks like Employee#full_name points to the superclass #name method and not at Employee#name! And this is the truth.

alias is a ruby keyword and is executed when source code gets parsed. When Boss class is parsed Boss#full_name is aliased to Boss#name and this will be truth for any Boss instance as per any Class instance that subclass Boss.

alias_method is instead, as the word says, a method and is executed in the current self scope.

### Using alias_method

class Boss
  def name
     p "Andrea"
  end
  def self.apply_alias
      alias_method :full_name, :name
  end
  apply_alias
end

class Employee < Boss
    def name
       p "Bob"
    end
    apply_alias
end

Boss.new.name # => Andrea
Boss.new.full_name # => Andrea
Employee.new.name # => Bob
Employee.new.full_name # => Bob

alias_method is executed the first time in the self scope of Boss and then the self scope of Employee and apply the method aliasing in the way we expected. While this looks like a tiny difference using alias_method grant more flexibility and may avoid unpredictable code behaviour.

Railo 4 beta released

Tue, 03 Jul 2012 09:40:00 +0200

Railo 4 beta has been released. See more info in Redis and I got really impressed by how is fast and reliable. Redis is a nosql key/value store quite commonly used in the Ruby world where I am digging these days. Many well known projects like Resque, Sidekiq and others make a wide use of Redis for storing any kind of data. What makes Redis really shining, over being so fast, is the ability to manage typed data like hashes, sets and lists. This feature makes the engine suitable to be used in many more situations than a plain key value store.

Looking around into the cfml community I discovered that Redis almost completely unknown in the coldfusion world. What a shame???

What a better occasion to make another cache extension for Railo. I have found in Jedis a very well designed and fully featured Java client and well… here we are.

The result is an extension that you can easily install on your Railo server. Once you got it running creating a new cache and use is very easy. You can find some more info on github.

I made some fast test comparing with the Railo MongoDb cache extension that I have developed as well.

<cfloop from="1" to="10" index="j">
	<cfset start = gettickcount()>
	<cfloop from="1" to="5000" index="i">
		<cfcache action="put" id="a#i#" value="#i#">
		<cfcache action="get" id="a#i#" name="v">
	</cfloop>
	<cfset end = gettickcount()>
	<cfset time = end -start>
	<cfset total = total + time >
	<cfoutput>#time/1000# s<br/></cfoutput>
	<cfflush>
    <cfset cacheClear()>
</cfloop>
<cfoutput>Average sec: #(total/10)/1000#</cfoutput>

This simple snippets runs 10 iterations with 5000 write/read operations each. ONn my machine MongoDb executes this in 3 secs average per iteration. Redis will do the same job in 1 sec average so basically 66% faster. I have reporetd the same performance even launching the scripts from 4 concurrents threads.

Resources

Rspec rails views tips

Sun, 22 Apr 2012 00:00:00 +0200

In the last days I faced some new challenge in testing rails views with rspec. Here is what I learned. I am useing the veru handy rails controller

#controller
before_filter :prepend_view_path_if_required

private
def prepend_view_path_if_required
    prepend_view_path "views/custom_view_path" if my_condition
end

When my_condition returns true, the passed path will placed on top of the views_path rails controller stack. Rails will then look into this location as first choice when looking for a view to render. This technique allows me to choose what view to render in different conditions without the need to hacks into my routes. Is a nice tool. You just say to rails: “if this condition is true looks for views here as a first choice. If nothing is found go on and check into the rest of the views locations stack”

What was a bit challenging was the view testing when prepend_view_path plays a role into the rendering process. Testing a single view placed for example in “views/custom_view_path/view_to_test” is not a big deal.

  # rspec view test
  it ....
    render "custom_view_path/view_to_test"
    rendered.should match/"text to match"/
  ....

But what if the view you are testing has code that render a partial?

#custom_view_path/view_to_test
render :partial => "users/login"

When you run it rspec creates a fake controller for you that will not know nothing about the ‘views path prepending’ that your application is expecting to use. The users/login partial will be looked for in “app/views/users” folder but not in “app/views/custom_view_path/users” where you may expect it is looked for. While a was trying to stub/mock things I then dicovereed that I could simply call the prepend_view_path method on the controller that rspec creates in views specs. My spec is looking like this:

# rspec view test
before do
    prepend_view_path "views/custom_view_path"
end

it ....
    render "custom_view_path/view_to_test"
    rendered.should match/"text to match"/
....

When I run my test the correct views path is added to the controller and my spec runs as expected. Simply and clean.

I then faced a new task. The application I am building uses an helper method called is_mobile_request? that check if the call comes from a mobile. A view I was testing is rendering a partial that isa actually using this same helper method. While this is a smell of a not perfect design I needed a way to stub the result of this helper method call so that my view was able to render as expected. My solution was to declare the spec of type helper and to stub the method on the helper scope.

# rspec view test
describe "custom_view_path/view_to_test.html.erb", type: :helper do

before do
    .....
    stub(helper).is_mobile_request?{false}
    ....
end

end

What happens here is that declaring the spec as type helper I can use/stub the helper scope in my spec.

Conclusion

While I use rspec every day I often find ways to learn new things about it. My advise is to look into rspec code/docs/groups before trying to create hacks to test things. Most of the times a clean and elegant solution is already baked into rspec just to solve your issue.

Ruby tip. Double association in a ternary operator

Thu, 09 Feb 2012 00:00:00 +0100

I found myself today in the opportunity to use a double association in a ternary operator. Basically I wanted to assign 2 variables based on a certain condition. Normal ruby double assignment works easy like this:

#simple double assignment
a,b = 1,2
a  # 1
b  # 2

Up to here no surprise. But what if I want to evaluate a condition and assigns both the variables??

#double assignment . Wrong ways
a,b = true ? (1,2) : (3,4)
a,b = true ? 1,2  : 3,4

Both this attempts ends in a syntax error. The right way:

a,b = false ? [1,2] : [3,4]
a  # 3
b  # 4

Easy and clean. Values comes in an array and are correctly assigned. Of course this works for any number of variables you need to assign

# triple assignment in ternary operator
a,b,c = false ? [1,2,3] : [4,5,6]
a   # 4
b   # 5
c   # 6

Rails migration redo

Sun, 29 Jan 2012 00:00:00 +0100

A very good practice, when writing a rails migration, is to be sure that the down methods works as expected. Once the migration is ready you should make it run and, if anything is fine, you should rollback it and run it again. In this way you are sure that both the up and down migration works as you planned. To make it faster you can use the following command

rake db:migrate:redo

This will run a rollback of the lates migration and will run it again.

You can even say how many steps you want to rollback and the re apply using the steps attributes as you do in a normal roll back command:

rake db:migrate:redo step=2

Monitor resque workers via upstart

Wed, 11 Jan 2012 00:00:00 +0100

Resque is a Redis-backed Ruby library for creating background jobs, placing them on multiple queues, and processing them later. Resque works firing up a set number of workers than will coninuously look for job to be processed and will execute them following the rules defined when the workers are launched. You can have a single worker process any queue or having a single worker processing a single queue or even fire up more workers that will look forward to a single queue. Read more about resque. A very important topic is to be sure that the workers are always available and that any time your application is deployed also the workers are restarted. This is crucial cause the worker load an instance of your application and will then process the jobs against this application instance. If you deploy a new version of your application but you do not restart your workers the jobs will be run on an outdated code version and this is clearly something you do not want!

I have solved both the issues using upstart to monitor the workers deamon and a rake task that kill all the workers any time the application is deployed. Basically the idea is that any time a worker is killed upstart will restart it with the most up to date application version.

Here is a sample of the upstart script I use to monitor the workers for my application: