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.

The list of modules to include is a little different: