2 min read

Today's Dispatch: Weaning ActionView off of content negotiation

I've been on "vacation" for the past few days and haven't had a ton of time to work on continuing refactoring, but I did manage to make a bit more progress, and I figured I'd share.

In Rails 2.2, ActionView had a fair number of content negotiation responsibilities. In particular, ActionView::Base had a method called template_format that looked like this:

def template_format
  if defined? @template_format
  elsif controller && controller.respond_to?(:request)
    @template_format = controller.request.format.to_sym
    @template_format = :html

Effectively, ActionView had a small content-negotiation responsibility, that effectively entailed grabbing the first acceptable format and using that. Thankfully, this particular piece of code was usually supplanted by Rails' respond_to code, which performed more proper content-negotiation, and set the template format directly: @response.template.template_format = mime_type.to_sym.

A number of other places, including the JavaScript helpers, also set the template format directly, which meant that for the most part, the template_format method was just a placeholder that was set by other parts of the system. However, there were more than a few cases where the default template_format method was getting used. Instead of having ActionView call back into the request object, I wanted to modify ActionView's initialization so that ActionController could feed it the list of acceptable formats directly.

With some work, I was able to make the modification, weaning ActionView off of content negotiation. Interestingly, this is related to my overall refactoring, which currently uses a four-element tuple to represent templates (path, acceptable extensions, prefix, and a partial flag). Now that ActionView uses a list of formats internally, it can easily pass them into find_by_parts, which handles figuring out which template to use based on the ordered list of extensions.

Just a note: find_by_parts is almost certainly a temporary creation to facilitate refactoring. However, for the purposes of refactoring, it is a convenient way to make sure that all pieces of the puzzle are passing around compatible values. In the end, we will probably be passing around Template objects, and this intermediate refactoring step will help us get all of our ducks in a row so that we can get there.

A fun diff that sort of demonstrates where I'm going with this is:

   def _pick_partial_template(partial_path) #:nodoc:
-    if partial_path.include?('/')
-      path = File.join(File.dirname(partial_path), "_#{File.basename(partial_path)}")
-    elsif controller
-      path = "#{controller.class.controller_path}/_#{partial_path}"
-    else
-      path = "_#{partial_path}"
+    unless partial_path.include?('/')
+      prefix = (controller && controller.class.controller_path)
-    self.view_paths.find_template(path, self.template_format)
+    view_paths.find_by_parts(partial_path, formats, prefix, true)

As you can see, by normalizing all path lookup to the four-element tuple, which includes merging path prefixes and appending "_" to partials, I was able to remove almost all of the code of this method, now requiring only that the controller_path is specified as a prefix if no existing prefix is supplied as part of the path (that's so render :partial => "foo" inside of the TestController looks for "test/_foo").

Again, I probably won't be able to do a ton of work over the next few days, but I'll blog when I can. Happy holidays folks!