From 21b5bb7f49ab7e8874b6718dcc0b1f633b3638ab Mon Sep 17 00:00:00 2001 From: Rodrigo Souto Date: Mon, 4 Aug 2014 21:04:36 +0000 Subject: [PATCH] profile-suggestions: rewrite suggestions fetch --- app/models/profile_suggestion.rb | 170 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------- lib/profile_suggestions_job.rb | 7 ++----- test/unit/profile_suggestion_test.rb | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 275 insertions(+), 97 deletions(-) diff --git a/app/models/profile_suggestion.rb b/app/models/profile_suggestion.rb index ebeedca..4fb1257 100644 --- a/app/models/profile_suggestion.rb +++ b/app/models/profile_suggestion.rb @@ -31,7 +31,7 @@ class ProfileSuggestion < ActiveRecord::Base end RULES = %w[ - friends_of_friends + people_with_common_friends people_with_common_communities people_with_common_tags communities_with_common_friends @@ -48,108 +48,106 @@ class ProfileSuggestion < ActiveRecord::Base COMMON_FRIENDS = 2 # Number of communities in common - COMMON_COMMUNITIES = 1 + COMMON_COMMUNITIES = 2 # Number of friends in common COMMON_TAGS = 2 - def self.friends_of_friends(person) - person_attempts = 0 - person_friends = person.friends - person_friends.each do |friend| - friend.friends.includes.each do |friend_of_friend| - person_attempts += 1 - return unless person.profile_suggestions.count < N_SUGGESTIONS && person_attempts < MAX_ATTEMPTS - unless friend_of_friend == person || friend_of_friend.is_a_friend?(person) || person.already_request_friendship?(friend_of_friend) - common_friends = friend_of_friend.friends & person_friends - if common_friends.size >= COMMON_FRIENDS - suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(friend_of_friend.id) - suggestion.common_friends = common_friends.size - suggestion.save - end - end - end + def self.register_suggestions(person, suggested_profiles, rule) + counter = rule.split(/.*_with_/).last + suggested_profiles.find_each do |suggested_profile| + suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(suggested_profile.id) + suggestion.send(counter+'=', suggested_profile.common_count.to_i) + suggestion.save! end end - def self.people_with_common_communities(person) - person_attempts = 0 - person_communities = person.communities - person_communities.each do |community| - community.members.each do |member| - person_attempts += 1 - return unless person.profile_suggestions.count < N_SUGGESTIONS && person_attempts < MAX_ATTEMPTS - unless member == person || member.is_a_friend?(person) || person.already_request_friendship?(member) - common_communities = person_communities & member.communities - if common_communities.size >= COMMON_COMMUNITIES - suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(member.id) - suggestion.common_communities = common_communities.size - suggestion.save - end - end - end + def self.calculate_suggestions(person) + ProfileSuggestion::RULES.each do |rule| + register_suggestions(person, ProfileSuggestion.send(rule, person), rule) end end + # If you are about to rewrite the following sql queries, think twice. After + # that make sure that whatever you are writing to replace it should be faster + # than how it is now. Yes, sqls are ugly but are fast! And fast is what we + # need here. + + def self.people_with_common_friends(person) + person_friends = person.friends.map(&:id) + person.environment.people. + select("profiles.*, suggestions.count AS common_count"). + joins(" + INNER JOIN (SELECT person_id, count(person_id) FROM + friendships WHERE friend_id IN (#{person_friends.join(',')}) AND + person_id NOT IN (#{(person_friends << person.id).join(',')}) + GROUP BY person_id + HAVING count(person_id) >= #{COMMON_FRIENDS}) AS suggestions + ON profiles.id = suggestions.person_id") + end + + def self.people_with_common_communities(person) + person_communities = person.communities.map(&:id) + person.environment.people. + select("profiles.*, suggestions.count AS common_count"). + joins(" + INNER JOIN (SELECT common_members.accessor_id, count(common_members.accessor_id) FROM + (SELECT DISTINCT accessor_id, resource_id FROM + role_assignments WHERE role_assignments.resource_id IN (#{person_communities.join(',')}) AND + role_assignments.accessor_id != #{person.id} AND role_assignments.resource_type = 'Profile' AND + role_assignments.accessor_type = 'Profile') AS common_members + GROUP BY common_members.accessor_id + HAVING count(common_members.accessor_id) >= #{COMMON_COMMUNITIES}) + AS suggestions ON profiles.id = suggestions.accessor_id") + end + def self.people_with_common_tags(person) - person_attempts = 0 - tags = person.article_tags.keys - tags.each do |tag| - person_attempts += 1 - return unless person.profile_suggestions.count < N_SUGGESTIONS && person_attempts < MAX_ATTEMPTS - tagged_content = ActsAsTaggableOn::Tagging.includes(:taggable).where(:tag_id => ActsAsTaggableOn::Tag.find_by_name(tag)) - tagged_content.each do |tg| - author = tg.taggable.created_by - unless author.nil? || author == person || author.is_a_friend?(person) || person.already_request_friendship?(author) - common_tags = tags & author.article_tags.keys - if common_tags.size >= COMMON_TAGS - suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(author.id) - suggestion.common_tags = common_tags.size - suggestion.save - end - end - end - end + profile_tags = person.articles.select('tags.id').joins(:tags).map(&:id) + person.environment.people. + select("profiles.*, suggestions.count as common_count"). + joins(" + INNER JOIN ( + SELECT results.profiles_id as profiles_id, count(results.profiles_id) FROM ( + SELECT DISTINCT tags.id, profiles.id as profiles_id FROM profiles + INNER JOIN articles ON articles.profile_id = profiles.id + INNER JOIN taggings ON taggings.taggable_id = articles.id AND taggings.context = ('tags') AND taggings.taggable_type = 'Article' + INNER JOIN tags ON tags.id = taggings.tag_id + WHERE (tags.id in (#{profile_tags.join(',')}) AND profiles.id != #{person.id})) AS results + GROUP BY results.profiles_id + HAVING count(results.profiles_id) >= #{COMMON_TAGS}) + as suggestions on profiles.id = suggestions.profiles_id") end def self.communities_with_common_friends(person) - community_attempts = 0 - person_friends = person.friends - person_friends.each do |friend| - friend.communities.each do |community| - community_attempts += 1 - return unless person.profile_suggestions.count < N_SUGGESTIONS && community_attempts < MAX_ATTEMPTS - unless person.is_member_of?(community) || community.already_request_membership?(person) - common_friends = community.members & person.friends - if common_friends.size >= COMMON_FRIENDS - suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(community.id) - suggestion.common_friends = common_friends.size - suggestion.save - end - end - end - end + person_friends = person.friends.map(&:id) + person.environment.communities. + select("profiles.*, suggestions.count AS common_count"). + joins(" + INNER JOIN (SELECT common_communities.resource_id, count(common_communities.resource_id) FROM + (SELECT DISTINCT accessor_id, resource_id FROM + role_assignments WHERE role_assignments.accessor_id IN (#{person_friends.join(',')}) AND + role_assignments.accessor_id != #{person.id} AND role_assignments.resource_type = 'Profile' AND + role_assignments.accessor_type = 'Profile') AS common_communities + GROUP BY common_communities.resource_id + HAVING count(common_communities.resource_id) >= #{COMMON_FRIENDS}) + AS suggestions ON profiles.id = suggestions.resource_id") end def self.communities_with_common_tags(person) - community_attempts = 0 - tags = person.article_tags.keys - tags.each do |tag| - community_attempts += 1 - return unless person.profile_suggestions.count < N_SUGGESTIONS && community_attempts < MAX_ATTEMPTS - tagged_content = ActsAsTaggableOn::Tagging.includes(:taggable).where(:tag_id => ActsAsTaggableOn::Tag.find_by_name(tag)) - tagged_content.each do |tg| - profile = tg.taggable.profile - unless !profile.community? || person.is_member_of?(profile) || profile.already_request_membership?(person) - common_tags = tags & profile.article_tags.keys - if common_tags.size >= COMMON_TAGS - suggestion = person.profile_suggestions.find_or_initialize_by_suggestion_id(profile.id) - suggestion.common_tags = common_tags.size - suggestion.save - end - end - end - end + profile_tags = person.articles.select('tags.id').joins(:tags).map(&:id) + person.environment.communities. + select("profiles.*, suggestions.count AS common_count"). + joins(" + INNER JOIN ( + SELECT results.profiles_id AS profiles_id, count(results.profiles_id) FROM ( + SELECT DISTINCT tags.id, profiles.id AS profiles_id FROM profiles + INNER JOIN articles ON articles.profile_id = profiles.id + INNER JOIN taggings ON taggings.taggable_id = articles.id AND taggings.context = ('tags') AND taggings.taggable_type = 'Article' + INNER JOIN tags ON tags.id = taggings.tag_id + WHERE (tags.id IN (#{profile_tags.join(',')}) AND profiles.id != #{person.id})) AS results + GROUP BY results.profiles_id + HAVING count(results.profiles_id) >= #{COMMON_TAGS}) + AS suggestions ON profiles.id = suggestions.profiles_id") end def disable diff --git a/lib/profile_suggestions_job.rb b/lib/profile_suggestions_job.rb index dc6271a..0d17c6e 100644 --- a/lib/profile_suggestions_job.rb +++ b/lib/profile_suggestions_job.rb @@ -12,13 +12,10 @@ class ProfileSuggestionsJob < Struct.new(:person_id) logger = Delayed::Worker.logger begin person = Person.find(person_id) - - ProfileSuggestion::RULES.each do |rule| - ProfileSuggestion.send(rule, person) - end + ProfileSuggestion.calculate_suggestions(person) UserMailer.profiles_suggestions_email(person).deliver rescue Exception => exception - logger.warn("Error with suggestions for person ID %d: %s" % [person_id, exception.to_s]) + logger.error("Error with suggestions for person ID %d: %s" % [person_id, exception.to_s]) end end diff --git a/test/unit/profile_suggestion_test.rb b/test/unit/profile_suggestion_test.rb index 14aee8d..3d099d9 100644 --- a/test/unit/profile_suggestion_test.rb +++ b/test/unit/profile_suggestion_test.rb @@ -38,6 +38,189 @@ class ProfileSuggestionTest < ActiveSupport::TestCase assert_equal true, repeated_suggestion.errors[:suggestion_id.to_s].present? end + should 'calculate people with common friends' do + p1 = create_user('testuser1').person + p2 = create_user('testuser2').person + p3 = create_user('testuser3').person + p4 = create_user('testuser4').person + p5 = create_user('testuser4').person + p6 = create_user('testuser4').person + p7 = create_user('testuser4').person + + p1.add_friend(p2) ; p2.add_friend(p1) + p1.add_friend(p3) ; p3.add_friend(p1) + p1.add_friend(p4) ; p2.add_friend(p3) + p3.add_friend(p2) ; p4.add_friend(p1) + p2.add_friend(p5) ; p5.add_friend(p2) + p2.add_friend(p6) ; p6.add_friend(p2) + p3.add_friend(p5) ; p5.add_friend(p3) + p4.add_friend(p6) ; p6.add_friend(p4) + p2.add_friend(p7) ; p7.add_friend(p2) + + assert_equivalent ProfileSuggestion.people_with_common_friends(p1), [p5,p6] + end + + should 'calculate people with common_communities' do + c1 = fast_create(Community) + c2 = fast_create(Community) + c3 = fast_create(Community) + c4 = fast_create(Community) + p1 = create_user('testuser1').person + p2 = create_user('testuser2').person + p3 = create_user('testuser3').person + p4 = create_user('testuser4').person + p5 = create_user('testuser4').person + + c1.add_member(p1) + c1.add_member(p2) + c1.add_member(p3) + c2.add_member(p1) + c2.add_member(p2) + c2.add_member(p4) + c3.add_member(p1) + c3.add_member(p4) + c4.add_member(p5) + + assert_equivalent ProfileSuggestion.people_with_common_communities(p1), [p2,p4] + end + + should 'calculate people with common_tags' do + p1 = create_user('testuser1').person + a11 = fast_create(Article, :profile_id => p1.id) + a11.tag_list = ['free software', 'veganism'] + a11.save! + a12 = fast_create(Article, :profile_id => p1.id) + a12.tag_list = ['anarchism'] + a12.save! + p2 = create_user('testuser2').person + a21 = fast_create(Article, :profile_id => p2.id) + a21.tag_list = ['free software'] + a21.save! + a22 = fast_create(Article, :profile_id => p2.id) + a22.tag_list = ['veganism'] + a22.save! + p3 = create_user('testuser3').person + a31 = fast_create(Article, :profile_id => p3.id) + a31.tag_list = ['anarchism'] + a31.save! + a32 = fast_create(Article, :profile_id => p3.id) + a32.tag_list = ['veganism'] + a32.save! + p4 = create_user('testuser4').person + a41 = fast_create(Article, :profile_id => p4.id) + a41.tag_list = ['free software', 'marxism'] + a41.save! + a42 = fast_create(Article, :profile_id => p4.id) + a42.tag_list = ['free software', 'vegetarianism',] + a42.save! + p5 = create_user('testuser4').person + a51 = fast_create(Article, :profile_id => p5.id) + a51.tag_list = ['proprietary software'] + a51.save! + a52 = fast_create(Article, :profile_id => p5.id) + a52.tag_list = ['onivorism', 'facism'] + a52.save! + + assert_equivalent ProfileSuggestion.people_with_common_tags(p1), [p2, p3] + end + + should 'calculate communities with common_friends' do + c1 = fast_create(Community) + c2 = fast_create(Community) + c3 = fast_create(Community) + c4 = fast_create(Community) + p1 = create_user('testuser1').person + p2 = create_user('testuser2').person + p3 = create_user('testuser3').person + p4 = create_user('testuser4').person + p5 = create_user('testuser4').person + + p1.add_friend(p2) + p1.add_friend(p3) + p1.add_friend(p4) + + c1.add_member(p2) + c1.add_member(p3) + c2.add_member(p2) + c2.add_member(p4) + c3.add_member(p2) + c4.add_member(p3) + + assert_equivalent ProfileSuggestion.communities_with_common_friends(p1), [c1,c2] + end + + should 'calculate communities with common_tags' do + p1 = create_user('testuser1').person + a11 = fast_create(Article, :profile_id => p1.id) + a11.tag_list = ['free software', 'veganism'] + a11.save! + a12 = fast_create(Article, :profile_id => p1.id) + a12.tag_list = ['anarchism'] + a12.save! + p2 = fast_create(Community) + a21 = fast_create(Article, :profile_id => p2.id) + a21.tag_list = ['free software'] + a21.save! + a22 = fast_create(Article, :profile_id => p2.id) + a22.tag_list = ['veganism'] + a22.save! + p3 = fast_create(Community) + a31 = fast_create(Article, :profile_id => p3.id) + a31.tag_list = ['anarchism'] + a31.save! + a32 = fast_create(Article, :profile_id => p3.id) + a32.tag_list = ['veganism'] + a32.save! + p4 = fast_create(Community) + a41 = fast_create(Article, :profile_id => p4.id) + a41.tag_list = ['free software', 'marxism'] + a41.save! + a42 = fast_create(Article, :profile_id => p4.id) + a42.tag_list = ['free software', 'vegetarianism',] + a42.save! + p5 = fast_create(Community) + a51 = fast_create(Article, :profile_id => p5.id) + a51.tag_list = ['proprietary software'] + a51.save! + a52 = fast_create(Article, :profile_id => p5.id) + a52.tag_list = ['onivorism', 'facism'] + a52.save! + + assert_equivalent ProfileSuggestion.communities_with_common_tags(p1), [p2, p3] + end + + should 'register suggestions' do + person = create_user('person').person + rule = ProfileSuggestion::RULES.sample + p1 = fast_create(Profile) + p2 = fast_create(Profile) + p3 = fast_create(Profile) + # Hack to simulate a common_count that generated on the rules + suggestions = Profile.select('profiles.*, profiles.id as common_count').where("id in (#{[p1,p2,p3].map(&:id).join(',')})") + + assert_difference 'ProfileSuggestion.count', 3 do + ProfileSuggestion.register_suggestions(person, suggestions, rule) + end + assert_no_difference 'ProfileSuggestion.count' do + s1 = ProfileSuggestion.find_or_initialize_by_suggestion_id(p1.id) + assert_equal p1, s1.suggestion + s2 = ProfileSuggestion.find_or_initialize_by_suggestion_id(p2.id) + assert_equal p2, s2.suggestion + s3 = ProfileSuggestion.find_or_initialize_by_suggestion_id(p3.id) + assert_equal p3, s3.suggestion + end + end + + should 'calculate every rule suggestion' do + person = create_user('person').person + ProfileSuggestion::RULES.each do |rule| + suggestion = fast_create(Profile) + ProfileSuggestion.expects(rule).returns([suggestion]) + ProfileSuggestion.expects(:register_suggestions).with(person, [suggestion], rule).returns(true) + end + ProfileSuggestion.calculate_suggestions(person) + end + should 'update existing person suggestion when the number of common friends increase' do suggested_person = create_user('test_user').person suggestion = ProfileSuggestion.create(:person => person, :suggestion => suggested_person, :common_friends => 2) @@ -56,8 +239,8 @@ class ProfileSuggestionTest < ActiveSupport::TestCase suggested_person.add_friend friend3 assert_difference 'ProfileSuggestion.find(suggestion.id).common_friends', 1 do - ProfileSuggestion.friends_of_friends(person) - end + ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_friends(person), 'people_with_common_friends') + end end should 'update existing person suggestion when the number of common communities increase' do @@ -72,7 +255,7 @@ class ProfileSuggestionTest < ActiveSupport::TestCase community2.add_member suggested_person assert_difference 'ProfileSuggestion.find(suggestion.id).common_communities', 1 do - ProfileSuggestion.people_with_common_communities(person) + ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_communities(person), 'people_with_common_communities') end end @@ -84,7 +267,7 @@ class ProfileSuggestionTest < ActiveSupport::TestCase create(Article, :created_by => suggested_person, :profile => suggested_person, :tag_list => 'first-tag, second-tag, third-tag') assert_difference 'ProfileSuggestion.find(suggestion.id).common_tags', 2 do - ProfileSuggestion.people_with_common_tags(person) + ProfileSuggestion.register_suggestions(person, ProfileSuggestion.people_with_common_tags(person), 'people_with_common_tags') end end @@ -101,7 +284,7 @@ class ProfileSuggestionTest < ActiveSupport::TestCase community.add_member member2 assert_difference 'ProfileSuggestion.find(suggestion.id).common_friends', 1 do - ProfileSuggestion.communities_with_common_friends(person) + ProfileSuggestion.register_suggestions(person, ProfileSuggestion.communities_with_common_friends(person), 'communities_with_common_friends') end end @@ -115,7 +298,7 @@ class ProfileSuggestionTest < ActiveSupport::TestCase create(Article, :created_by => other_person, :profile => community, :tag_list => 'first-tag, second-tag, third-tag') assert_difference 'ProfileSuggestion.find(suggestion.id).common_tags', 2 do - ProfileSuggestion.communities_with_common_tags(person) + ProfileSuggestion.register_suggestions(person, ProfileSuggestion.communities_with_common_tags(person), 'communities_with_common_tags') end end end -- libgit2 0.21.2