Don’t call controller from background job, please. Do it differently!

Photo remix available thanks to the courtesy of francescomucio. CC BY 2.0

Some time ago while doing Code Review for one of our clients I stumbled on a very unexpected piece of code. Background job calling a controller.

class OrderPdfJob
  def self.perform(order_id, locale)
    controller = PdfController.new
    pdf = controller.generate_pdf_for_order(order_id, locale)
    OrderPdf.create file: pdf, order: order
    # ...
  end
end

class PdfController < ApplicationController
  skip_before_filter :authenticate_user!

  def generate_pdf_for_order(order_id, locale)
    # ...
  end
end

I was a bit speechless. Calling controller out of HTTP request/response loop sounds like an odd thing to do. I had to investigate and refactor.

As you can probably imagine, it turned out that the PDF was generated based on HTML, and the HTML was generated by the controller. After all, controllers in Rails can render views, that’s one of their responsibilities. But they also take care of HTTP request parsing, managing cookies, session, authentication, authorization, content negotiation building respone, and all the stuff necessary for your webapp to work.

But when you call a controller from background job, in a situation like this, none of this things happens. What you actually want is not a Controller but just a Renderer. If we can achieve that, we won’t need to have this ugly skip_before_filter in the code, and our intentions are going to be way more clear for the rest of the team reading the code.

Rails, render me this

After a few moments of struggling with rails, reading the doc, and trying things in irb, it turned out that all we need is this.

# app/renderers/pdf_renderer.rb
require 'pdfkit'

class PdfRenderer < ActionController::Metal
  include ActionController::Helpers
  include ActionController::Rendering
  include ActionController::Renderers::All
  include AbstractController::Layouts

  append_view_path "app/views"

  View = Struct.new(:layout, :action, :locals)

  def render_pdf(view)
    html = render_to_string(
      layout: view.layout,
      action: view.action,
      locals: view.locals
    )
    kit  = PDFKit.new(html, {
      page_size:     'A4',
      margin_top:    '0.5in',
      margin_bottom: '0.5in',
      margin_left:   '0.5in',
      margin_right:  '0.5in',
    })
    kit.to_pdf
  end
end

# app/renderers/order_pdf_renderer.rb
class OrderPdfRenderer < PdfRenderer
  helper :orders

  def generate_pdf_for_order(order, locale)
    I18n.with_locale(locale) do
      render_pdf(View.new.tap do |v|
        v.layout = "pdf"
        v.action = "order.html.erb"
        v.locals = {order: order}
      end)
    end
  end
end

The view for the rendered pdf is in app/views/order_pdf_renderer/order.html.erb file. The layout in app/views/layouts/pdf.html.erb. Nothing surprising here. And if you prefer presenters/decorators over helpers you can just remove include ActionController::Helpers and helper :orders and have even less code to maintain.

During the refactoring this part of code I also extracted a separate layer responsible for getting all the data required to generate the PDF. As a result the renderer is called with order and not just order_id. That makes testing everything easier.

We could probably try to go even further and use less and less of what we don’t need from Rails in such situation. Perhaps there is a clean way of using ActionView part for such purpose without the coupling to controllers and HTTP-context at all?

Summary

I hope this technique can be a useful tool in your refactoring controllers toolbox . Whenever you stumble upon a controller methods that are not used to deal with HTTP requests-response loop, think about extracting them into a separate object and giving a proper name. Even if they relay on Rails controllers features, taking from Rails what you just need, might be easier than you think.

Update for Rails 4.1

The list of modules to include is a little different:

class PdfRenderer < ActionController::Metal
  include ActionView::ViewPaths
  include AbstractController::Rendering
  include AbstractController::Helpers
  include ActionController::Helpers
  include ActionView::Rendering
  include ActionView::Layouts
  include ActionController::Rendering
  include ActionController::Renderers
  include ActionController::Renderers::All
end