Finder Objects in Rails

Article summary

ActiveRecord is perhaps the most compelling component of Ruby on Rails. ActiveRecord makes it easy to create a record, or to find one, or even to migrate the database schema. Unfortunately those “find” methods can be a little too seductive. Let’s see why.

Our example project is a basic social network. It will start with profiles and friends, and add from there. We’ve implemented “Profile” and “Friend” features and now our customer asks for “Search by State”. In our “Profiles” controller, we implement:

1 2 3 4 5 
class ProfilesController   def search     @profiles = Profile.find_all_by_state(params[:state])   end end

At first, this is wonderful. It does not burden the caller with unnecessary knowledge, and keeps the code simple and clean. It’s also simple to test through either state-based or interaction testing. At the customer’s request, we add an admin listing, a friend list, and the profile page itself. It’s working beautifully.

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 
class ProfilesController   def search     @profiles = Profile.find_all_by_state(params[:state])   end    def show     @profile = Profile.find(params[:id])   end end  class ProfilesAdminController   def index     @profiles = Profile.find(:all)   end end  class FriendsController   def index     @user = User.find(params[:user_id])     @friends = @user.friends   end end

Then our customer comes back and says we need the ability to have private profiles. A profile should only show up if it’s public. We make the changes as follows:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 
class ProfilesController   def search     @profiles = Profile.find_all_by_state_and_public(params[:state], true)   end    def show     @profile = Profile.find(params[:id], :conditions => {:public => true})   end end  class ProfilesAdminController   def index     @profiles = Profile.find(:all)   end end  class FriendsController   def index     @user = User.find(params[:user_id])     @friends = @user.friends.find_all_by_public(true)   end end

We know it’s not ‘DRY’, but we move on in the interest of ‘Getting Things Done’. We run our integration tests, but one is failing. A user can no longer view their own profile. After investigating, we realize that in reality the rule should be “A profile should be visible if it’s public or belongs to the current user”.

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 
class ProfilesController   def search     conditions = [       "state = ? and (public = ? or user_id = ?)",        params[:state], true, current_user.id     ]     @profiles = Profile.find(:all, :conditions => conditions)   end    def show     conditions = ["public = ? or user_id = ?", true, current_user.id]     @profile = Profile.find(params[:id], :conditions => conditions)   end end  class ProfilesAdminController   def search     @profiles = Profile.find(:all)   end end  class FriendsController   def index     @user = User.find(params[:user_id])     conditions = ["public = ? or user_id = ?", true, current_user.id]     @friends = @user.friends.find(:all, :conditions => conditions)   end end

This has gotten out of hand. It’s just not ok to leave SQL lying around in our controller, even if it’s just condition arrays. The caller should specify intent, but not worry about details like privacy.

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 
class Profile < ActiveRecord::Base   def self.find_all_by_state(state, current_user)     conditions = [       "state = ? and (public = ? or user_id = ?)",        state, true, current_user.id     ]     find(:all, :conditions => conditions)   end      def self.find_all_for_admin     find(:all)   end      def self.find_by_id(id, current_user)     conditions = ["public = ? or user_id = ?", true, current_user.id]     find(id, :conditions => conditions)   end      def self.find_friend_profiles_of(user, current_user)     conditions = ["public = ? or user_id = ?", true, current_user.id]     user.friend_profiles.find(:all, :conditions => conditions)   end end  class ProfilesController   def search     @profiles = Profile.find_all_by_state(params[:state], current_user)   end    def show     @profile = Profile.find_by_id(params[:id], current_user)   end end  class ProfilesAdminController   def search     @profiles = Profile.find_all_for_admin   end end  class FriendsController   def index     @user = User.find(params[:user_id])     @friends = Profile.find_friend_profiles_of(@user, current_user)   end end

Now all of the actions that need profiles can find them safely by calling one of our methods, and the privacy logic is handled for us. Something doesn’t feel quite right, but we’ll try it for a bit to make sure we’re not just suffering from indigestion.

A month later, the customer asks for a different kind of search. Now we need to search by last name. By this time, there’s a second pair on the project, and they implement this feature.

1 2 3 4 5 6 7 8 9 10 11 12 13 
class ProfilesController   def search_by_state     @profiles = Profile.find_all_by_state(params[:state], current_user)   end    def search_by_last_name     @profiles = Profile.find_all_by_last_name(params[:last_name])   end    def show     @profile = Profile.find_by_id(params[:id], current_user)   end end

Their integration test passes and they check it in. After we deploy, the customer complains. It seems the new form of search doesn’t take privacy into account. Why did this happen? Intelligent programmers following project conventions and acting conscientiously will still make these mistakes. Is it a communication breakdown? I contend that it’s a code flaw. When finding rules are implemented, they need to be designed to allow later programmers to forget the details until they need them. The programmers should not be filling their minds with privacy while implementing the controller action. But how?

In my experience, finder logic very often gets complicated very quickly. The responsibility for finding a specific kind of entity is a big deal, and should not be given to a class that already fills another responsibility (persisting profile information). In addition, finder methods should not be implicit, as it leads to the issues outlined above. Instead we use finder objects.

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 
class ProfileFinder   attr_accessor :current_user    def find_all_by_state(state)     conditions = [       "state = ? and (public = ? or user_id = ?)",        state, true, @current_user.id     ]     Profile.find(:all, :conditions => conditions)   end    def find_all_by_last_name(last_name)     conditions = [       "last_name = ? and (public = ? or user_id = ?)",       last_name, true, @current_user.id     ]     Profile.find(:all, :conditions => conditions)   end      def find_all_for_admin     Profile.find(:all)   end      def find_by_id(id)     conditions = ["public = ? or user_id = ?", true, @current_user.id]     Profile.find(id, :conditions => conditions)   end      def find_friend_profiles_of(user)     conditions = ["public = ? or user_id = ?", true, @current_user.id]     user.friend_profiles.find(:all, :conditions => conditions)   end end  class ProfilesController   before_filter :build_profile_finder    def search_by_state     @profiles = @profile_finder.find_all_by_state(params[:state])   end    def search_by_last_name     @profiles = @profile_finder.find_all_by_last_name(params[:last_name])   end    def show     @profile = @profile_finder.find_by_id(params[:id])   end    private     def build_profile_finder     @profile_finder = ProfileFinder.new     @profile_finder.current_user = current_user   end end

Now when our new pair is asked to implement the ability to find profiles by last name, they know project standards dictate the use of the relevant finder. Their integration test fails since the method does not exist. When they create it, they see the other privacy logic in close proximity, and now have enough information to initiate a conversation with the customer about privacy. We’ve found that conscientious use of finder objects enables us to have very consistent finding logic, which reduces bugs and user confusion. It also simplifies tests, as the tests benefit from a clear scope definition, and are not cluttered by other model logic. It also makes finder rules discoverable as they are now in a centralized location.

We decided that finder objects are allowed to know model fields and conditions, sql, etc. They are also allowed to call with_scope on that model. This all fits with their responsibility of knowing how to find the appropriate model.

Usage Tips

For bonus points, combine this with dependency injection. We do, and we’re loving it. It allows context like the current user to be injected, and then the caller doesn’t even need to know that the method requires the current user, as that’s simply a privacy constraint.

We’ve also discovered that for purposes of authorization rules (are you allowed to see this record?), it helps for the caller to provide intent. If a record otherwise exists but the user does not have permission to follow through with the stated intent, nil is returned.

1 2 
profile_finder.find_by_id(id, :for => :show) profile_finder.find_by_id(id, :for => :update)

I may be able to see my friend’s profile, but I can’t update it. Finder objects already need some authorization information to restrict queries. It makes sense to block out access at the data layer. If I can’t fetch the record, I can’t update it. My code can be required to provide an intent with the :for attribute; you may rig your finder to raise an error if no intention is indicated.

Sometimes, even just the act of determining the permissions of the current user and the object at hand can be complicated, and given that you may need to apply action-oriented permissions for an entity type in more than one place in your code, it makes sense to implement a Permitter (in this case, a ProfilePermitter). The topic of permitters may warrant another blog post.

Conversation
  • Daniel Haran says:

    has_finder /named_scope is far more elegant to declare findders. See:
    http://didcoe.id.au/archives/edge-rails-has_finder-merged-to-edge

  • Paul Barry says:

    Depending on the level of complexity, I think this could be more elegany than has_finder/named_scope. It could also be used in conjunction with has_finder/named_scope. I like the idea of injecting the current user into the finder. I think that you might have equally complicated logic in terms of updates, so it might make sense to refer to the Finder more generically as a Service, so it can handle CRUD.

  • ActsAsFlinn says:

    @Paul, I can’t see how finder objects are more elegant. named_scope is significantly easier to read and probably more compact and reusable. Just in terms of efficiency finder objects don’t make sense. If you try to chain any of those finders together you get multiple queries. named_scope uses with_scope for one query.

    
    class Profile < ActiveRecord::Base
      cattr_accessor :current_user
    
      named_scope :by_state, lamda{ |state| :conditions => { :state => state }}
      named_scope :by_last_name, lamda{ |last_name| :conditions => { :last_name => last_name }}
      named_scope :for_current_user_or_public, :conditions => ["public = ? or user_id = ?", true, @@current_user.id]
    end
    
    class ProfilesController < ApplicationController
      before_filter :set_current_user
    
      def search_by_state
        @profiles = Profile.for_current_user_or_public.by_state(params[:state]).all
      end
    
      def search_by_last_name
        @profiles = Profile.for_current_user_or_public.by_last_name(params[:last_name]).all
      end
    
      def show
        @profile = Profile.for_current_user_or_public.find(params[:id])
      end
    
      protected
        def set_current_user
          Profile.current_user = current_user
        end
    end
    
  • Ryan Fogle says:

    The idea with finders is to make it so any chaining required is done inside the finder, to keep it to a single query. Named scopes are a decent solution. However, they still spread the logic a little too far afield.

    My current project recently felt this pain. We deal heavily with events. 18 one-week iterations into the project, the customer asked for privacy support for events. We have many controllers that find one or more events. Each one needed to be updated to support privacy. Fixing this was very painful, and it was difficult to prove that every piece of the application respects privacy.

    We chose to move to finders, because when all of the application uses the EventFinder, it’s trivial to prove that it respects privacy. In addition, the details are hidden from the caller. Privacy becomes an implicit rule that automatically applies throughout the application, instead of an explicit call that can be neglected.

    With that said, for a different project without stringent data access rules, finders may be overkill. Unfortunately, you may not always know for sure at the beginning of the project.

  • ActsAsFlinn says:

    @Ryan, that makes sense. I’ve used scoping extensively and have run into situations where that can backfire. On a recent project we scoped all finders a certain way only to find later on in the project an edge case that needs to be unscoped. Less code can sometimes be rigid. That said, I still think scoping is more efficient querying the DB and a little more DRY in comparison especially if you’re explicitly defining finders.

    
    class Profile < ActiveRecord::Base
      cattr_accessor :current_user
    
      class << self
        def current_user_or_public_scope(&block)
          with_scope(:find => { :conditions => ["public = ? or user_id = ?", true, @@current_user.id] }) do
            yield
          end
        end
    
        def find_all_by_state(state)
          current_user_or_public_scope do
            find(:all, :conditions => { :state => state })
          end
        end
    
        def find_all_by_last_name(last_name)
          current_user_or_public_scope do
            find(:all, :conditions => { :last_name => last_name })
          end
        end
      end
    end
    
  • Justin says:

    Finder objects separate your data model (in this instance an activerecord object) from the business logic of your application. A finder uses the data model to find objects given a specific rule or set of rules. I think it is wrong to have a data model use itself to find objects given a specific rule or set of rules.

    I know rails allows you to do it, in fact, it encourages you to do so. However, I think we need to step back and think before we drink that rails software design “koolaid”.

  • Comments are closed.