Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-add follow recommendations API #7918

Merged
merged 3 commits into from
Jul 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions app/controllers/api/v1/suggestions_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# frozen_string_literal: true

class Api::V1::SuggestionsController < Api::BaseController
include Authorization

before_action -> { doorkeeper_authorize! :read }
before_action :require_user!
before_action :set_accounts

respond_to :json

def index
render json: @accounts, each_serializer: REST::AccountSerializer
end

private

def set_accounts
@accounts = PotentialFriendshipTracker.get(current_account.id, limit: limit_param(DEFAULT_ACCOUNTS_LIMIT))
end
end
39 changes: 39 additions & 0 deletions app/lib/potential_friendship_tracker.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# frozen_string_literal: true

class PotentialFriendshipTracker
EXPIRE_AFTER = 90.days.seconds
MAX_ITEMS = 80

WEIGHTS = {
reply: 1,
favourite: 10,
reblog: 20,
}.freeze

class << self
def record(account_id, target_account_id, action)
key = "interactions:#{account_id}"
weight = WEIGHTS[action]

redis.zincrby(key, weight, target_account_id)
redis.zremrangebyrank(key, 0, -MAX_ITEMS)
redis.expire(key, EXPIRE_AFTER)
end

def remove(account_id, target_account_id)
redis.zrem("interactions:#{account_id}", target_account_id)
end

def get(account_id, limit: 20, offset: 0)
account_ids = redis.zrevrange("interactions:#{account_id}", offset, limit)
return [] if account_ids.empty?
Account.searchable.where(id: account_ids)
end

private

def redis
Redis.current
end
end
end
29 changes: 1 addition & 28 deletions app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ class Account < ApplicationRecord
scope :matches_username, ->(value) { where(arel_table[:username].matches("#{value}%")) }
scope :matches_display_name, ->(value) { where(arel_table[:display_name].matches("#{value}%")) }
scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) }
scope :searchable, -> { where(suspended: false).where(moved_to_account_id: nil) }

delegate :email,
:unconfirmed_email,
Expand Down Expand Up @@ -309,34 +310,6 @@ def inboxes
DeliveryFailureTracker.filter(urls)
end

def triadic_closures(account, limit: 5, offset: 0)
sql = <<-SQL.squish
WITH first_degree AS (
SELECT target_account_id
FROM follows
WHERE account_id = :account_id
)
SELECT accounts.*
FROM follows
INNER JOIN accounts ON follows.target_account_id = accounts.id
WHERE
account_id IN (SELECT * FROM first_degree)
AND target_account_id NOT IN (SELECT * FROM first_degree)
AND target_account_id NOT IN (:excluded_account_ids)
AND accounts.suspended = false
GROUP BY target_account_id, accounts.id
ORDER BY count(account_id) DESC
OFFSET :offset
LIMIT :limit
SQL

excluded_account_ids = account.excluded_from_timeline_account_ids + [account.id]

find_by_sql(
[sql, { account_id: account.id, excluded_account_ids: excluded_account_ids, limit: limit, offset: offset }]
)
end

def search_for(terms, limit = 10)
textsearch, query = generate_query_for_search(terms)

Expand Down
12 changes: 12 additions & 0 deletions app/models/concerns/account_interactions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,21 +89,27 @@ def follow!(other_account, reblogs: nil, uri: nil)
.find_or_create_by!(target_account: other_account)

rel.update!(show_reblogs: reblogs)
remove_potential_friendship(other_account)

rel
end

def block!(other_account, uri: nil)
remove_potential_friendship(other_account)
block_relationships.create_with(uri: uri)
.find_or_create_by!(target_account: other_account)
end

def mute!(other_account, notifications: nil)
notifications = true if notifications.nil?
mute = mute_relationships.create_with(hide_notifications: notifications).find_or_create_by!(target_account: other_account)
remove_potential_friendship(other_account)

# When toggling a mute between hiding and allowing notifications, the mute will already exist, so the find_or_create_by! call will return the existing Mute without updating the hide_notifications attribute. Therefore, we check that hide_notifications? is what we want and set it if it isn't.
if mute.hide_notifications? != notifications
mute.update!(hide_notifications: notifications)
end

mute
end

Expand Down Expand Up @@ -194,4 +200,10 @@ def lists_for_local_distribution
lists.joins(account: :user)
.where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago)
end

private

def remove_potential_friendship(other_account)
PotentialFriendshipTracker.remove(id, other_account.id)
end
end
8 changes: 8 additions & 0 deletions app/services/favourite_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ def call(account, status)
return favourite unless favourite.nil?

favourite = Favourite.create!(account: account, status: status)

create_notification(favourite)
bump_potential_friendship(account, status)

favourite
end

Expand All @@ -33,6 +36,11 @@ def create_notification(favourite)
end
end

def bump_potential_friendship(account, status)
return if account.following?(status.account_id)
PotentialFriendshipTracker.record(account.id, status.account_id, :favourite)
end

def build_json(favourite)
Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(
favourite,
Expand Down
7 changes: 7 additions & 0 deletions app/services/post_status_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ def call(account, text, in_reply_to = nil, **options)
redis.setex("idempotency:status:#{account.id}:#{options[:idempotency]}", 3_600, status.id)
end

bump_potential_friendship(account, status)

status
end

Expand Down Expand Up @@ -79,4 +81,9 @@ def process_hashtags_service
def redis
Redis.current
end

def bump_potential_friendship(account, status)
return if !status.reply? || account.following?(status.account_id)
PotentialFriendshipTracker.record(account.id, status.in_reply_to_account_id, :reply)
end
end
7 changes: 7 additions & 0 deletions app/services/reblog_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ def call(account, reblogged_status)
ActivityPub::DistributionWorker.perform_async(reblog.id)

create_notification(reblog)
bump_potential_friendship(account, reblog)

reblog
end

Expand All @@ -41,6 +43,11 @@ def create_notification(reblog)
end
end

def bump_potential_friendship(account, reblog)
return if account.following?(reblog.reblog.account_id)
PotentialFriendshipTracker.record(account.id, reblog.reblog.account_id, :reblog)
end

def build_json(reblog)
Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(
reblog,
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@

resources :streaming, only: [:index]
resources :custom_emojis, only: [:index]
resources :suggestions, only: [:index]

get '/search', to: 'search#index', as: :search

Expand Down
35 changes: 35 additions & 0 deletions spec/controllers/api/v1/suggestions_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
require 'rails_helper'

RSpec.describe Api::V1::SuggestionsController, type: :controller do
render_views

let(:user) { Fabricate(:user) }
let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read write') }

before do
allow(controller).to receive(:doorkeeper_token) { token }
end

describe 'GET #index' do
let(:bob) { Fabricate(:account) }
let(:jeff) { Fabricate(:account) }

before do
PotentialFriendshipTracker.record(user.account_id, bob.id, :reblog)
PotentialFriendshipTracker.record(user.account_id, jeff.id, :favourite)

get :index
end

it 'returns http success' do
expect(response).to have_http_status(200)
end

it 'returns accounts' do
json = body_as_json

expect(json.size).to be >= 1
expect(json.map { |i| i[:id] }).to include *[bob, jeff].map { |i| i.id.to_s }
end
end
end
71 changes: 0 additions & 71 deletions spec/models/account_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -454,77 +454,6 @@
end
end

describe '.triadic_closures' do
let!(:me) { Fabricate(:account) }
let!(:friend) { Fabricate(:account) }
let!(:friends_friend) { Fabricate(:account) }
let!(:both_follow) { Fabricate(:account) }

before do
me.follow!(friend)
friend.follow!(friends_friend)

me.follow!(both_follow)
friend.follow!(both_follow)
end

it 'finds accounts you dont follow which are followed by accounts you do follow' do
expect(described_class.triadic_closures(me)).to eq [friends_friend]
end

it 'limits by 5 with offset 0 by defualt' do
first_degree = 6.times.map { Fabricate(:account) }
matches = 5.times.map { Fabricate(:account) }
first_degree.each { |account| me.follow!(account) }
matches.each do |match|
first_degree.each { |account| account.follow!(match) }
first_degree.shift
end

expect(described_class.triadic_closures(me)).to eq matches
end

it 'accepts arbitrary limits' do
another_friend = Fabricate(:account)
higher_friends_friend = Fabricate(:account)
me.follow!(another_friend)
friend.follow!(higher_friends_friend)
another_friend.follow!(higher_friends_friend)

expect(described_class.triadic_closures(me, limit: 1)).to eq [higher_friends_friend]
end

it 'acceps arbitrary offset' do
another_friend = Fabricate(:account)
higher_friends_friend = Fabricate(:account)
me.follow!(another_friend)
friend.follow!(higher_friends_friend)
another_friend.follow!(higher_friends_friend)

expect(described_class.triadic_closures(me, offset: 1)).to eq [friends_friend]
end

context 'when you block account' do
before do
me.block!(friends_friend)
end

it 'rejects blocked accounts' do
expect(described_class.triadic_closures(me)).to be_empty
end
end

context 'when you mute account' do
before do
me.mute!(friends_friend)
end

it 'rejects muted accounts' do
expect(described_class.triadic_closures(me)).to be_empty
end
end
end

describe '#statuses_count' do
subject { Fabricate(:account) }

Expand Down