From 5d2a6e3994c7d5af3c5974413e65946418f736d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Victor=20Cruz=20Duen=CC=83as?= <xtream23@gmail.com>
Date: Sat, 25 May 2013 08:50:39 +0200
Subject: [PATCH] refactored and tested AvatarLookup

less array copying

Avoid N queries
---
 lib/avatar_lookup.rb                  | 39 +++++---------
 spec/components/avatar_lookup_spec.rb | 78 +++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 25 deletions(-)
 create mode 100644 spec/components/avatar_lookup_spec.rb

diff --git a/lib/avatar_lookup.rb b/lib/avatar_lookup.rb
index 3f3edfcbca47a..532b3d005b0a3 100644
--- a/lib/avatar_lookup.rb
+++ b/lib/avatar_lookup.rb
@@ -1,35 +1,24 @@
 class AvatarLookup
+  attr_accessor :user_ids, :users
 
-  def initialize(user_ids)
-    @user_ids = user_ids
-
-    @user_ids.flatten!
-    @user_ids.compact! if @user_ids.present?
-    @user_ids.uniq! if @user_ids.present?
-
-    @loaded = false
+  def initialize(user_ids=[])
+    self.user_ids = AvatarLookup.filtered_users(user_ids)
   end
 
   # Lookup a user by id
   def [](user_id)
-    ensure_loaded!
-    @users_hashed[user_id]
+    self.users = AvatarLookup.hashed_users(user_ids) if self.users.nil?
+    self.users[user_id]
   end
 
+  private
+  def self.filtered_users(user_ids=[])
+    user_ids.flatten.tap(&:compact!).tap(&:uniq!)
+  end
 
-  protected
-
-    def ensure_loaded!
-      return if @loaded
-
-      @users_hashed = {}
-      # need email for hash
-      User.where(id: @user_ids).select([:id, :email, :email, :username]).each do |u|
-        @users_hashed[u.id] = u
-      end
-
-      @loaded = true
-    end
-
-
+  def self.hashed_users(user_ids=[])
+    users = User.where(:id => user_ids).select([:id, :email, :username])
+    users_with_ids = users.collect {|x| [x.id, x] }.flatten
+    Hash[*users_with_ids]
+  end
 end
diff --git a/spec/components/avatar_lookup_spec.rb b/spec/components/avatar_lookup_spec.rb
new file mode 100644
index 0000000000000..a2a9e719462a4
--- /dev/null
+++ b/spec/components/avatar_lookup_spec.rb
@@ -0,0 +1,78 @@
+# encoding: utf-8
+
+require 'spec_helper'
+require 'avatar_lookup'
+
+describe AvatarLookup do
+  let!(:user){ Fabricate(:user) }
+  user_ids = [1, 2]
+
+  describe '#new' do
+    before do
+      AvatarLookup.stubs(:filtered_users).once.returns(user_ids)
+      @avatar_lookup = AvatarLookup.new
+    end
+
+    it 'init with cleaned user ids' do
+      @avatar_lookup.user_ids.should eq(user_ids)
+    end
+
+    it 'init users hash' do
+      @avatar_lookup.users.should eq(nil)
+    end
+  end
+
+  describe '#[]' do
+    before do
+      @avatar_lookup = AvatarLookup.new([user.id])
+    end
+
+    it 'returns nil if user_id does not exists' do
+      @avatar_lookup[0].should be_nil
+    end
+
+    it 'returns nil if user_id is nil' do
+      @avatar_lookup[nil].should be_nil
+    end
+
+    it 'returns user if user_id exists' do
+      @avatar_lookup[user.id].should eq(user)
+    end
+  end
+
+  describe '.filtered_users' do
+    it 'returns empty array if no params' do
+      AvatarLookup.filtered_users.should eq([])
+    end
+
+    it 'returns empty array' do
+      AvatarLookup.filtered_users([]).should eq([])
+    end
+
+    it 'returns filtered ids' do
+      AvatarLookup.filtered_users(user_ids).should eq(user_ids)
+    end
+
+    it 'returns flatten filtered ids' do
+      AvatarLookup.filtered_users([1, [2]]).should eq(user_ids)
+    end
+
+    it 'returns compact filtered ids' do
+      AvatarLookup.filtered_users([1, 2, nil]).should eq(user_ids)
+    end
+
+    it 'returns uniq filtered ids' do
+      AvatarLookup.filtered_users([1, 2, 2]).should eq(user_ids)
+    end
+  end
+
+  describe '.hashed_users' do
+    it 'returns empty hash if no params' do
+      AvatarLookup.hashed_users.should eq({})
+    end
+
+    it 'returns hashed users by id' do
+      AvatarLookup.hashed_users([user.id]).should eq({user.id => user})
+    end
+  end
+end
\ No newline at end of file