The Rise and Fall of a Personal Cargo-Cult

Writing good software has never been easier thanks to the other developers that have put free, high-quality libraries out on the internet. The price of free is in understanding how different libraries interact, which often do so in complicated ways leading to the creation of cargo-cults. Like the Melanesians, we do things not because we know why, but because it’s worked in the past.

Nearly everyone in software has been guilty of following along with an established pattern when confronted with new and unfamiliar code, despite any weakness in the underlying explanation. This is not that story. This is a story about how I (somewhat embarrassingly) created my own cargo-cult for a library I had been using for years and the excessively complicated approach I used to diffuse it.

The Problem

This particular example is in a Ruby On Rails project, but it can happen anywhere in any language.

We use Cancancan as our authorization framework. Cancancan includes a convenient helper load_and_authorize_resource, which saves multiple lines of code per controller method, along with ensuring that nobody accidentally skips authorizing the user.

A few months ago I needed to write a controller method for a custom endpoint with a route similar to the code snippet below:

    resources :things do 
        get '/special_thing' => 'things#get_special_thing'
    end

I used load_and_authorize_resource like I normally do, but instead of getting an instance of Thing, @thing was nil.

I looked at some documentation and came up with the following, that still didn’t work:

class ThingsController < ApplicationController
    load_and_authorize_resource only: [:get_special_thing], id: :thing_id

    def get_special_thing
        Rails.logger.info("Here's the thing: #{@thing}") # @thing is nil. What's wrong?
    end
end

It was late, I was under a deadline, and someone mentioned that in another situation, changing GET to PUT got different results. API purity was preventing me from going to bed, so a PUT request it became. It worked, though I had no idea why. I added the remaining methods as PUT requests and the cargo-cult was born.

The Right Solution

Over the next few months, we repeated the pattern of using the id option for load_and_authorize_resource along with PUT requests. I brushed off questions from colleagues with saying things like “It’s just the way it is. Rails is magic.”

The features got out to production and the customers were happy. Unless they opened the network inspector and analyzed every call, the feature worked flawlessly.

Still, I knew that one day we’d need to expose this via a public API and I would face even more questions about this oddity. I had a little downtime and decided to dig in.

After a little more Google-fu failed to turn up any answers, I did the most logical thing: turn to the source. I found the source location from bundler and used grep -r to drill down into the following line of code in lib/cancan/controller_resource.rb:

def member_action?
  new_actions.include?(@params[:action].to_sym) || @options[:singleton] || ( (@params[:id] || @params[@options[:id_param]]) && !collection_actions.include?(@params[:action].to_sym))
end

I clearly have an :id, clearly something else was wrong. I added a few binding.pry_remote calls to introspect further. (FYI, the command gem pristine is useful if you ever end up down that path).

All was then revealed with how the complicated interactions between libraries come into play. If you run rake routes you’ll see that the actual route is:

/things/:thing_id/special_thing

In a POST or a PUT Rails translates thing_id to id and the code for member_action? returns true. But for a GET Rails doesn’t do that and member_action? returns false.

The id: :thing_id I had in my example above was effectively a no-op. What the controller should have been is this:

class ThingsController < ApplicationController
    # load_and_authorize_resource only: [:get_special_thing], id: :thing_id # NOT THIS
    load_and_authorize_resource only: [:get_special_thing], id_param: :thing_id # THIS

    def get_special_thing
        Rails.logger.info("Here's the thing: #{@thing}")
    end
end

Armed with this newfound knowledge, I set out to fix all the controllers using this broken pattern that we had built over the years. Which is when I came to a depressing realization. Right up until the point where I started doing it wrong, we had been using the correct solution. Oof. If I had copy-and-pasted from one of these files initially, this blog post would not exist.

Conclusions

I could have not written this blog post. I probably should have not written this blog post, but separated from this mistake I think it highlights one of the challenges we have as software developers. In any non-trivial projects there’s an enormous amount of knowledge required to understand it fully, but that we can get by not knowing on a day to day basis. If perfect is truly the enemy of good, that’s okay.

So to close, some truths:

1. “Wrong” code can still be working code (in production, customers happy).
2. With open-source, the answer to almost anything can eventually be figured out
3. If Google doesn’t know your problem, you’ve done something so strange that it’s probably the result of a trivial typo
4. Rails can feel magical, but there is deterministic code underneath it all
5. After a point in time, the amount of software knowledge you have forgotten will outweigh the amount you retain.

Daniel Burka

Related Posts: