From 6d105e5c09b9b9431cd8422aa69bb9aa428916be Mon Sep 17 00:00:00 2001
From: Raphael Sofaer <raphael@joindiaspora.com>
Date: Sat, 4 Jun 2011 22:34:48 -0700
Subject: [PATCH] Don't n query in UsersController#public

---
 app/models/status_message.rb              |  7 ++--
 lib/diaspora/ostatus_builder.rb           |  6 +---
 spec/lib/diaspora/ostatus_builder_spec.rb | 41 ++++++++++++++---------
 3 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/app/models/status_message.rb b/app/models/status_message.rb
index 3fb1f9b7fa..7a68134275 100644
--- a/app/models/status_message.rb
+++ b/app/models/status_message.rb
@@ -93,13 +93,14 @@ class StatusMessage < Post
     identifiers.empty? ? [] : Person.where(:diaspora_handle => identifiers)
   end
 
-  def to_activity
+  def to_activity(opts={})
+    author = opts[:author] || self.author #Use an already loaded author if passed in.
     <<-XML
   <entry>
     <title>#{x(self.formatted_message(:plain_text => true))}</title>
     <content>#{x(self.formatted_message(:plain_text => true))}</content>
-    <link rel="alternate" type="text/html" href="#{self.author.url}p/#{self.id}"/>
-    <id>#{self.author.url}p/#{self.id}</id>
+    <link rel="alternate" type="text/html" href="#{author.url}p/#{self.id}"/>
+    <id>#{author.url}p/#{self.id}</id>
     <published>#{self.created_at.xmlschema}</published>
     <updated>#{self.updated_at.xmlschema}</updated>
     <activity:verb>http://activitystrea.ms/schema/1.0/post</activity:verb>
diff --git a/lib/diaspora/ostatus_builder.rb b/lib/diaspora/ostatus_builder.rb
index e5fd0e8557..b13674b217 100644
--- a/lib/diaspora/ostatus_builder.rb
+++ b/lib/diaspora/ostatus_builder.rb
@@ -63,11 +63,7 @@ module Diaspora
     def create_body
       @posts.inject("") do |xml,curr|
         if curr.respond_to?(:to_activity)
-          unless xml
-            curr.to_activity
-          else
-            xml + curr.to_activity
-          end
+          xml + curr.to_activity(:author => @user.person)
         else
           xml
         end
diff --git a/spec/lib/diaspora/ostatus_builder_spec.rb b/spec/lib/diaspora/ostatus_builder_spec.rb
index 1250565cc8..c4fde6bbe7 100644
--- a/spec/lib/diaspora/ostatus_builder_spec.rb
+++ b/spec/lib/diaspora/ostatus_builder_spec.rb
@@ -8,32 +8,43 @@ require File.join(Rails.root,  'lib/diaspora/ostatus_builder')
 
 describe Diaspora::OstatusBuilder do
 
-  let!(:user) { alice }
-  let(:aspect) { user.aspects.first }
-  let!(:public_status_messages) {
-    3.times.inject([]) do |arr,n|
-      s = user.post(:status_message, :text => "hey#{n}", :public => true, :to => aspect.id)
+  before do
+    @aspect = alice.aspects.first
+    @public_status_messages = 3.times.inject([]) do |arr,n|
+      s = alice.post(:status_message, :text => "hey#{n}", :public => true, :to => @aspect.id)
       arr << s
     end
-  }
-  let!(:private_status_messages) {
-    3.times.inject([]) do |arr,n|
-      s = user.post(:status_message, :text => "secret_ney#{n}", :public => false, :to => aspect.id)
+
+    @private_status_messages = 3.times.inject([]) do |arr,n|
+      s = alice.post(:status_message, :text => "secret_ney#{n}", :public => false, :to => @aspect.id)
       arr << s
     end
-  }
-  let!(:atom) { director = Diaspora::Director.new; director.build(Diaspora::OstatusBuilder.new(user, public_status_messages)) }
+
+    director = Diaspora::Director.new;
+    @atom = director.build(Diaspora::OstatusBuilder.new(alice, @public_status_messages))
+  end
 
   it 'should include a users posts' do
-    public_status_messages.each{ |status| atom.should include status.text}
+    @public_status_messages.each{ |status| @atom.should include status.text}
   end
 
   it 'should iterate through all objects, and not stop if it runs into a post without a to_activity' do
-    messages = public_status_messages.collect{|x| x.text}
-    public_status_messages.insert(1, [])
+    messages = @public_status_messages.collect{|x| x.text}
+    @public_status_messages.insert(1, [])
     director = Diaspora::Director.new;
-    atom2 = director.build(Diaspora::OstatusBuilder.new(user, public_status_messages))
+    atom2 = director.build(Diaspora::OstatusBuilder.new(alice, @public_status_messages))
     messages.each{ |message| atom2.should include message }
   end
+
+    include Oink::InstanceTypeCounter
+  it 'does not query the db for the author of every post' do
+    alice.person #Preload user.person
+    ActiveRecord::Base.reset_instance_type_count
+    director = Diaspora::Director.new
+    messages = StatusMessage.where(:author_id => alice.person.id, :public => true)
+    builder = Diaspora::OstatusBuilder.new(alice, messages)
+    director.build( builder )
+    report_hash["Person"].should be_nil #No people should have been instantiated
+  end
 end
 
-- 
GitLab