Download as
Browse Code »

Commit 21b5bb7f49ab7e8874b6718dcc0b1f633b3638ab

Authored by Rodrigo Souto
1 parent 0dbfb391
Exists in master and in 21 other branches activate_plugin, api_fixies, api_tasks, article_blog_visualization_format, article_layout_refactoring, article_layout_temp, blog_page_bug, clone_article, communities_ratings, design_template, event_layout, fix_forum, fix_optional_fields, fix_suggest_article, folder_improvments, manage_organization, news_page, script_to_update, template_mirror_block, tests, tests_fixies

profile-suggestions: rewrite suggestions fetch 

Fetching suggestions with straight sql queries in order to improve
performance.

Also replacing log.warn by log.error on profile_suggestions job.
Showing 3 changed files with 275 additions and 97 deletions   Show diff stats
app/models/profile_suggestion.rb
  Diff comments   View file @21b5bb7
... ... @@ -31,7 +31,7 @@ class ProfileSuggestion < ActiveRecord::Base
31 31 end
32 32  
33 33 RULES = %w[
34   - friends_of_friends
  34 + people_with_common_friends
35 35 people_with_common_communities
36 36 people_with_common_tags
37 37 communities_with_common_friends
... ... @@ -48,108 +48,106 @@ class ProfileSuggestion < ActiveRecord::Base
48 48 COMMON_FRIENDS = 2
49 49  
50 50 # Number of communities in common
51   - COMMON_COMMUNITIES = 1
  51 + COMMON_COMMUNITIES = 2
52 52  
53 53 # Number of friends in common
54 54 COMMON_TAGS = 2
55 55  
56   - def self.friends_of_friends(person)
57   - person_attempts = 0
58   - person_friends = person.friends
59   - person_friends.each do |friend|
60   - friend.friends.includes.each do |friend_of_friend|
61   - person_attempts += 1
62   - return unless person.profile_suggestions.count < N_SUGGESTIONS && person_attempts < MAX_ATTEMPTS
63   - unless friend_of_friend == person || friend_of_friend.is_a_friend?(person) || person.already_request_friendship?(friend_of_friend)
64   - common_friends = friend_of_friend.friends & person_friends
65   - if common_friends.size >= COMMON_FRIENDS
66   - suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(friend_of_friend.id)
67   - suggestion.common_friends = common_friends.size
68   - suggestion.save
69   - end
70   - end
71   - end
  56 + def self.register_suggestions(person, suggested_profiles, rule)
  57 + counter = rule.split(/.*_with_/).last
  58 + suggested_profiles.find_each do |suggested_profile|
  59 + suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(suggested_profile.id)
  60 + suggestion.send(counter+'=', suggested_profile.common_count.to_i)
  61 + suggestion.save!
72 62 end
73 63 end
74 64  
75   - def self.people_with_common_communities(person)
76   - person_attempts = 0
77   - person_communities = person.communities
78   - person_communities.each do |community|
79   - community.members.each do |member|
80   - person_attempts += 1
81   - return unless person.profile_suggestions.count < N_SUGGESTIONS && person_attempts < MAX_ATTEMPTS
82   - unless member == person || member.is_a_friend?(person) || person.already_request_friendship?(member)
83   - common_communities = person_communities & member.communities
84   - if common_communities.size >= COMMON_COMMUNITIES
85   - suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(member.id)
86   - suggestion.common_communities = common_communities.size
87   - suggestion.save
88   - end
89   - end
90   - end
  65 + def self.calculate_suggestions(person)
  66 + ProfileSuggestion::RULES.each do |rule|
  67 + register_suggestions(person, ProfileSuggestion.send(rule, person), rule)
91 68 end
92 69 end
93 70  
  71 + # If you are about to rewrite the following sql queries, think twice. After
  72 + # that make sure that whatever you are writing to replace it should be faster
  73 + # than how it is now. Yes, sqls are ugly but are fast! And fast is what we
  74 + # need here.
  75 +
  76 + def self.people_with_common_friends(person)
  77 + person_friends = person.friends.map(&:id)
  78 + person.environment.people.
  79 + select("profiles.*, suggestions.count AS common_count").
  80 + joins("
  81 + INNER JOIN (SELECT person_id, count(person_id) FROM
  82 + friendships WHERE friend_id IN (#{person_friends.join(',')}) AND
  83 + person_id NOT IN (#{(person_friends << person.id).join(',')})
  84 + GROUP BY person_id
  85 + HAVING count(person_id) >= #{COMMON_FRIENDS}) AS suggestions
  86 + ON profiles.id = suggestions.person_id")
  87 + end
  88 +
  89 + def self.people_with_common_communities(person)
  90 + person_communities = person.communities.map(&:id)
  91 + person.environment.people.
  92 + select("profiles.*, suggestions.count AS common_count").
  93 + joins("
  94 + INNER JOIN (SELECT common_members.accessor_id, count(common_members.accessor_id) FROM
  95 + (SELECT DISTINCT accessor_id, resource_id FROM
  96 + role_assignments WHERE role_assignments.resource_id IN (#{person_communities.join(',')}) AND
  97 + role_assignments.accessor_id != #{person.id} AND role_assignments.resource_type = 'Profile' AND
  98 + role_assignments.accessor_type = 'Profile') AS common_members
  99 + GROUP BY common_members.accessor_id
  100 + HAVING count(common_members.accessor_id) >= #{COMMON_COMMUNITIES})
  101 + AS suggestions ON profiles.id = suggestions.accessor_id")
  102 + end
  103 +
94 104 def self.people_with_common_tags(person)
95   - person_attempts = 0
96   - tags = person.article_tags.keys
97   - tags.each do |tag|
98   - person_attempts += 1
99   - return unless person.profile_suggestions.count < N_SUGGESTIONS && person_attempts < MAX_ATTEMPTS
100   - tagged_content = ActsAsTaggableOn::Tagging.includes(:taggable).where(:tag_id => ActsAsTaggableOn::Tag.find_by_name(tag))
101   - tagged_content.each do |tg|
102   - author = tg.taggable.created_by
103   - unless author.nil? || author == person || author.is_a_friend?(person) || person.already_request_friendship?(author)
104   - common_tags = tags & author.article_tags.keys
105   - if common_tags.size >= COMMON_TAGS
106   - suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(author.id)
107   - suggestion.common_tags = common_tags.size
108   - suggestion.save
109   - end
110   - end
111   - end
112   - end
  105 + profile_tags = person.articles.select('tags.id').joins(:tags).map(&:id)
  106 + person.environment.people.
  107 + select("profiles.*, suggestions.count as common_count").
  108 + joins("
  109 + INNER JOIN (
  110 + SELECT results.profiles_id as profiles_id, count(results.profiles_id) FROM (
  111 + SELECT DISTINCT tags.id, profiles.id as profiles_id FROM profiles
  112 + INNER JOIN articles ON articles.profile_id = profiles.id
  113 + INNER JOIN taggings ON taggings.taggable_id = articles.id AND taggings.context = ('tags') AND taggings.taggable_type = 'Article'
  114 + INNER JOIN tags ON tags.id = taggings.tag_id
  115 + WHERE (tags.id in (#{profile_tags.join(',')}) AND profiles.id != #{person.id})) AS results
  116 + GROUP BY results.profiles_id
  117 + HAVING count(results.profiles_id) >= #{COMMON_TAGS})
  118 + as suggestions on profiles.id = suggestions.profiles_id")
113 119 end
114 120  
115 121 def self.communities_with_common_friends(person)
116   - community_attempts = 0
117   - person_friends = person.friends
118   - person_friends.each do |friend|
119   - friend.communities.each do |community|
120   - community_attempts += 1
121   - return unless person.profile_suggestions.count < N_SUGGESTIONS && community_attempts < MAX_ATTEMPTS
122   - unless person.is_member_of?(community) || community.already_request_membership?(person)
123   - common_friends = community.members & person.friends
124   - if common_friends.size >= COMMON_FRIENDS
125   - suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(community.id)
126   - suggestion.common_friends = common_friends.size
127   - suggestion.save
128   - end
129   - end
130   - end
131   - end
  122 + person_friends = person.friends.map(&:id)
  123 + person.environment.communities.
  124 + select("profiles.*, suggestions.count AS common_count").
  125 + joins("
  126 + INNER JOIN (SELECT common_communities.resource_id, count(common_communities.resource_id) FROM
  127 + (SELECT DISTINCT accessor_id, resource_id FROM
  128 + role_assignments WHERE role_assignments.accessor_id IN (#{person_friends.join(',')}) AND
  129 + role_assignments.accessor_id != #{person.id} AND role_assignments.resource_type = 'Profile' AND
  130 + role_assignments.accessor_type = 'Profile') AS common_communities
  131 + GROUP BY common_communities.resource_id
  132 + HAVING count(common_communities.resource_id) >= #{COMMON_FRIENDS})
  133 + AS suggestions ON profiles.id = suggestions.resource_id")
132 134 end
133 135  
134 136 def self.communities_with_common_tags(person)
135   - community_attempts = 0
136   - tags = person.article_tags.keys
137   - tags.each do |tag|
138   - community_attempts += 1
139   - return unless person.profile_suggestions.count < N_SUGGESTIONS && community_attempts < MAX_ATTEMPTS
140   - tagged_content = ActsAsTaggableOn::Tagging.includes(:taggable).where(:tag_id => ActsAsTaggableOn::Tag.find_by_name(tag))
141   - tagged_content.each do |tg|
142   - profile = tg.taggable.profile
143   - unless !profile.community? || person.is_member_of?(profile) || profile.already_request_membership?(person)
144   - common_tags = tags & profile.article_tags.keys
145   - if common_tags.size >= COMMON_TAGS
146   - suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(profile.id)
147   - suggestion.common_tags = common_tags.size
148   - suggestion.save
149   - end
150   - end
151   - end
152   - end
  137 + profile_tags = person.articles.select('tags.id').joins(:tags).map(&:id)
  138 + person.environment.communities.
  139 + select("profiles.*, suggestions.count AS common_count").
  140 + joins("
  141 + INNER JOIN (
  142 + SELECT results.profiles_id AS profiles_id, count(results.profiles_id) FROM (
  143 + SELECT DISTINCT tags.id, profiles.id AS profiles_id FROM profiles
  144 + INNER JOIN articles ON articles.profile_id = profiles.id
  145 + INNER JOIN taggings ON taggings.taggable_id = articles.id AND taggings.context = ('tags') AND taggings.taggable_type = 'Article'
  146 + INNER JOIN tags ON tags.id = taggings.tag_id
  147 + WHERE (tags.id IN (#{profile_tags.join(',')}) AND profiles.id != #{person.id})) AS results
  148 + GROUP BY results.profiles_id
  149 + HAVING count(results.profiles_id) >= #{COMMON_TAGS})
  150 + AS suggestions ON profiles.id = suggestions.profiles_id")
153 151 end
154 152  
155 153 def disable
... ...
lib/profile_suggestions_job.rb
  Diff comments   View file @21b5bb7
... ... @@ -12,13 +12,10 @@ class ProfileSuggestionsJob &lt; Struct.new(:person_id)
12 12 logger = Delayed::Worker.logger
13 13 begin
14 14 person = Person.find(person_id)
15   -
16   - ProfileSuggestion::RULES.each do |rule|
17   - ProfileSuggestion.send(rule, person)
18   - end
  15 + ProfileSuggestion.calculate_suggestions(person)
19 16 UserMailer.profiles_suggestions_email(person).deliver
20 17 rescue Exception => exception
21   - logger.warn("Error with suggestions for person ID %d: %s" % [person_id, exception.to_s])
  18 + logger.error("Error with suggestions for person ID %d: %s" % [person_id, exception.to_s])
22 19 end
23 20 end
24 21  
... ...
test/unit/profile_suggestion_test.rb
  Diff comments   View file @21b5bb7
... ... @@ -38,6 +38,189 @@ class ProfileSuggestionTest &lt; ActiveSupport::TestCase
38 38 assert_equal true, repeated_suggestion.errors[:suggestion_id.to_s].present?
39 39 end
40 40  
  41 + should 'calculate people with common friends' do
  42 + p1 = create_user('testuser1').person
  43 + p2 = create_user('testuser2').person
  44 + p3 = create_user('testuser3').person
  45 + p4 = create_user('testuser4').person
  46 + p5 = create_user('testuser4').person
  47 + p6 = create_user('testuser4').person
  48 + p7 = create_user('testuser4').person
  49 +
  50 + p1.add_friend(p2) ; p2.add_friend(p1)
  51 + p1.add_friend(p3) ; p3.add_friend(p1)
  52 + p1.add_friend(p4) ; p2.add_friend(p3)
  53 + p3.add_friend(p2) ; p4.add_friend(p1)
  54 + p2.add_friend(p5) ; p5.add_friend(p2)
  55 + p2.add_friend(p6) ; p6.add_friend(p2)
  56 + p3.add_friend(p5) ; p5.add_friend(p3)
  57 + p4.add_friend(p6) ; p6.add_friend(p4)
  58 + p2.add_friend(p7) ; p7.add_friend(p2)
  59 +
  60 + assert_equivalent ProfileSuggestion.people_with_common_friends(p1), [p5,p6]
  61 + end
  62 +
  63 + should 'calculate people with common_communities' do
  64 + c1 = fast_create(Community)
  65 + c2 = fast_create(Community)
  66 + c3 = fast_create(Community)
  67 + c4 = fast_create(Community)
  68 + p1 = create_user('testuser1').person
  69 + p2 = create_user('testuser2').person
  70 + p3 = create_user('testuser3').person
  71 + p4 = create_user('testuser4').person
  72 + p5 = create_user('testuser4').person
  73 +
  74 + c1.add_member(p1)
  75 + c1.add_member(p2)
  76 + c1.add_member(p3)
  77 + c2.add_member(p1)
  78 + c2.add_member(p2)
  79 + c2.add_member(p4)
  80 + c3.add_member(p1)
  81 + c3.add_member(p4)
  82 + c4.add_member(p5)
  83 +
  84 + assert_equivalent ProfileSuggestion.people_with_common_communities(p1), [p2,p4]
  85 + end
  86 +
  87 + should 'calculate people with common_tags' do
  88 + p1 = create_user('testuser1').person
  89 + a11 = fast_create(Article, :profile_id => p1.id)
  90 + a11.tag_list = ['free software', 'veganism']
  91 + a11.save!
  92 + a12 = fast_create(Article, :profile_id => p1.id)
  93 + a12.tag_list = ['anarchism']
  94 + a12.save!
  95 + p2 = create_user('testuser2').person
  96 + a21 = fast_create(Article, :profile_id => p2.id)
  97 + a21.tag_list = ['free software']
  98 + a21.save!
  99 + a22 = fast_create(Article, :profile_id => p2.id)
  100 + a22.tag_list = ['veganism']
  101 + a22.save!
  102 + p3 = create_user('testuser3').person
  103 + a31 = fast_create(Article, :profile_id => p3.id)
  104 + a31.tag_list = ['anarchism']
  105 + a31.save!
  106 + a32 = fast_create(Article, :profile_id => p3.id)
  107 + a32.tag_list = ['veganism']
  108 + a32.save!
  109 + p4 = create_user('testuser4').person
  110 + a41 = fast_create(Article, :profile_id => p4.id)
  111 + a41.tag_list = ['free software', 'marxism']
  112 + a41.save!
  113 + a42 = fast_create(Article, :profile_id => p4.id)
  114 + a42.tag_list = ['free software', 'vegetarianism',]
  115 + a42.save!
  116 + p5 = create_user('testuser4').person
  117 + a51 = fast_create(Article, :profile_id => p5.id)
  118 + a51.tag_list = ['proprietary software']
  119 + a51.save!
  120 + a52 = fast_create(Article, :profile_id => p5.id)
  121 + a52.tag_list = ['onivorism', 'facism']
  122 + a52.save!
  123 +
  124 + assert_equivalent ProfileSuggestion.people_with_common_tags(p1), [p2, p3]
  125 + end
  126 +
  127 + should 'calculate communities with common_friends' do
  128 + c1 = fast_create(Community)
  129 + c2 = fast_create(Community)
  130 + c3 = fast_create(Community)
  131 + c4 = fast_create(Community)
  132 + p1 = create_user('testuser1').person
  133 + p2 = create_user('testuser2').person
  134 + p3 = create_user('testuser3').person
  135 + p4 = create_user('testuser4').person
  136 + p5 = create_user('testuser4').person
  137 +
  138 + p1.add_friend(p2)
  139 + p1.add_friend(p3)
  140 + p1.add_friend(p4)
  141 +
  142 + c1.add_member(p2)
  143 + c1.add_member(p3)
  144 + c2.add_member(p2)
  145 + c2.add_member(p4)
  146 + c3.add_member(p2)
  147 + c4.add_member(p3)
  148 +
  149 + assert_equivalent ProfileSuggestion.communities_with_common_friends(p1), [c1,c2]
  150 + end
  151 +
  152 + should 'calculate communities with common_tags' do
  153 + p1 = create_user('testuser1').person
  154 + a11 = fast_create(Article, :profile_id => p1.id)
  155 + a11.tag_list = ['free software', 'veganism']
  156 + a11.save!
  157 + a12 = fast_create(Article, :profile_id => p1.id)
  158 + a12.tag_list = ['anarchism']
  159 + a12.save!
  160 + p2 = fast_create(Community)
  161 + a21 = fast_create(Article, :profile_id => p2.id)
  162 + a21.tag_list = ['free software']
  163 + a21.save!
  164 + a22 = fast_create(Article, :profile_id => p2.id)
  165 + a22.tag_list = ['veganism']
  166 + a22.save!
  167 + p3 = fast_create(Community)
  168 + a31 = fast_create(Article, :profile_id => p3.id)
  169 + a31.tag_list = ['anarchism']
  170 + a31.save!
  171 + a32 = fast_create(Article, :profile_id => p3.id)
  172 + a32.tag_list = ['veganism']
  173 + a32.save!
  174 + p4 = fast_create(Community)
  175 + a41 = fast_create(Article, :profile_id => p4.id)
  176 + a41.tag_list = ['free software', 'marxism']
  177 + a41.save!
  178 + a42 = fast_create(Article, :profile_id => p4.id)
  179 + a42.tag_list = ['free software', 'vegetarianism',]
  180 + a42.save!
  181 + p5 = fast_create(Community)
  182 + a51 = fast_create(Article, :profile_id => p5.id)
  183 + a51.tag_list = ['proprietary software']
  184 + a51.save!
  185 + a52 = fast_create(Article, :profile_id => p5.id)
  186 + a52.tag_list = ['onivorism', 'facism']
  187 + a52.save!
  188 +
  189 + assert_equivalent ProfileSuggestion.communities_with_common_tags(p1), [p2, p3]
  190 + end
  191 +
  192 + should 'register suggestions' do
  193 + person = create_user('person').person
  194 + rule = ProfileSuggestion::RULES.sample
  195 + p1 = fast_create(Profile)
  196 + p2 = fast_create(Profile)
  197 + p3 = fast_create(Profile)
  198 + # Hack to simulate a common_count that generated on the rules
  199 + suggestions = Profile.select('profiles.*, profiles.id as common_count').where("id in (#{[p1,p2,p3].map(&:id).join(',')})")
  200 +
  201 + assert_difference 'ProfileSuggestion.count', 3 do
  202 + ProfileSuggestion.register_suggestions(person, suggestions, rule)
  203 + end
  204 + assert_no_difference 'ProfileSuggestion.count' do
  205 + s1 = ProfileSuggestion.find_or_initialize_by_suggestion_id(p1.id)
  206 + assert_equal p1, s1.suggestion
  207 + s2 = ProfileSuggestion.find_or_initialize_by_suggestion_id(p2.id)
  208 + assert_equal p2, s2.suggestion
  209 + s3 = ProfileSuggestion.find_or_initialize_by_suggestion_id(p3.id)
  210 + assert_equal p3, s3.suggestion
  211 + end
  212 + end
  213 +
  214 + should 'calculate every rule suggestion' do
  215 + person = create_user('person').person
  216 + ProfileSuggestion::RULES.each do |rule|
  217 + suggestion = fast_create(Profile)
  218 + ProfileSuggestion.expects(rule).returns([suggestion])
  219 + ProfileSuggestion.expects(:register_suggestions).with(person, [suggestion], rule).returns(true)
  220 + end
  221 + ProfileSuggestion.calculate_suggestions(person)
  222 + end
  223 +
41 224 should 'update existing person suggestion when the number of common friends increase' do
42 225 suggested_person = create_user('test_user').person
43 226 suggestion = ProfileSuggestion.create(:person => person, :suggestion => suggested_person, :common_friends => 2)
... ... @@ -56,8 +239,8 @@ class ProfileSuggestionTest &lt; ActiveSupport::TestCase
56 239 suggested_person.add_friend friend3
57 240  
58 241 assert_difference 'ProfileSuggestion.find(suggestion.id).common_friends', 1 do
59   - ProfileSuggestion.friends_of_friends(person)
60   - end
  242 + ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_friends(person), 'people_with_common_friends')
  243 + end
61 244 end
62 245  
63 246 should 'update existing person suggestion when the number of common communities increase' do
... ... @@ -72,7 +255,7 @@ class ProfileSuggestionTest &lt; ActiveSupport::TestCase
72 255 community2.add_member suggested_person
73 256  
74 257 assert_difference 'ProfileSuggestion.find(suggestion.id).common_communities', 1 do
75   - ProfileSuggestion.people_with_common_communities(person)
  258 + ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_communities(person), 'people_with_common_communities')
76 259 end
77 260 end
78 261  
... ... @@ -84,7 +267,7 @@ class ProfileSuggestionTest &lt; ActiveSupport::TestCase
84 267 create(Article, :created_by => suggested_person, :profile => suggested_person, :tag_list => 'first-tag, second-tag, third-tag')
85 268  
86 269 assert_difference 'ProfileSuggestion.find(suggestion.id).common_tags', 2 do
87   - ProfileSuggestion.people_with_common_tags(person)
  270 + ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_tags(person), 'people_with_common_tags')
88 271 end
89 272 end
90 273  
... ... @@ -101,7 +284,7 @@ class ProfileSuggestionTest &lt; ActiveSupport::TestCase
101 284 community.add_member member2
102 285  
103 286 assert_difference 'ProfileSuggestion.find(suggestion.id).common_friends', 1 do
104   - ProfileSuggestion.communities_with_common_friends(person)
  287 + ProfileSuggestion.register_suggestions(person, ProfileSuggestion.communities_with_common_friends(person), 'communities_with_common_friends')
105 288 end
106 289  
107 290 end
... ... @@ -115,7 +298,7 @@ class ProfileSuggestionTest &lt; ActiveSupport::TestCase
115 298 create(Article, :created_by => other_person, :profile => community, :tag_list => 'first-tag, second-tag, third-tag')
116 299  
117 300 assert_difference 'ProfileSuggestion.find(suggestion.id).common_tags', 2 do
118   - ProfileSuggestion.communities_with_common_tags(person)
  301 + ProfileSuggestion.register_suggestions(person, ProfileSuggestion.communities_with_common_tags(person), 'communities_with_common_tags')
119 302 end
120 303 end
121 304 end
... ...