ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

cross-shard filter conversations by user 

fixes CNVS-4625

test-plan:
 - create four shards A, B, C, and D.
 - create three users A, B, and C on shards A, B, and C, respectively.
 - create conversation on shard A between users A and B:
   - on shard A as user A, filter by user B
   - on shard B as user A, filter by user B
   - on shard C as user A, filter by user B
   - on shard B as user B, filter by user A
   - on shard C as user B, filter by user A
 - create conversation on shard A between users B and C:
   - on shard A as user B, filter by user C
   - on shard B as user B, filter by user C
   - on shard C as user B, filter by user C
   - on shard D as user B, filter by user C
 - create users D and E on shards A and B, respectively.
 - create conversation on shard A between users A and D:
   - on shard A as user A, filter by user D
   - on shard B as user A, filter by user D
 - create conversation on shard A between users B and E:
   - on shard B as user B, filter by user E
   - on shard C as user B, filter by user E
 - create conversation on shard B between users D and E:
   - on shard B as user E, filter by user D
 - create user F on shard A.
 - create conversation on shard B between users A and F:
   - on shard B as user A, filter by user F

Change-Id: If41a801f5b6b8a79c8fc0717b10293467ba92c19
Reviewed-on: https://gerrit.instructure.com/18522
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
QA-Review: Jacob Fugal <jacob@instructure.com>
This commit is contained in:
Jacob Fugal 2013-03-11 13:47:06 -06:00
parent 69c235ea57
commit 64a4a47699
4 changed files with 114 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

115
app/models/conversation_participant.rb
View File

@ -60,28 +60,98 @@ class ConversationParticipant < ActiveRecord::Base
{:conditions => ["conversation_participants.root_account_ids <> '' AND " + like_condition('?', root_account_id_matcher, false), id_string]}
}
# Produces a subscope for conversations in which the given users are
# participants (either all or any, depending on options[:mode]).
#
# The execution of subqueries and general complexity is due to the fact that
# the existence of a CP for any given user can only be guaranteed on the
# user's shard and the conversation's shard. To get a condition that can be
# applied on a single shard (for the scope being constructed) we basically
# have to execute this condition immediately and then just limit on the
# resulting ids into the scope we're building.
#
# Performance assumptions:
#
# * number of unique shards among given user tags is small (there will be one
# query per such shard)
# * the number of unique shards on which those users have conversations is
# relatively small (there will be one query per such shard)
# * number of found conversations is relatively small (each will be
# instantiated to get id)
#
tagged_scope_handler(/\Auser_(\d+)\z/) do |tags, options|
user_ids = tags.map{ |t| t.sub(/\Auser_/, '').to_i }
conditions = if options[:mode] == :or || tags.size == 1
[<<-SQL, user_ids]
EXISTS (
SELECT *
FROM conversation_participants cp
WHERE cp.conversation_id = conversation_participants.conversation_id
AND user_id IN (?)
)
SQL
else
[<<-SQL, user_ids, user_ids.size]
(
SELECT COUNT(*)
FROM conversation_participants cp
WHERE cp.conversation_id = conversation_participants.conversation_id
AND user_id IN (?)
) = ?
SQL
scope_shard = scope(:find, :shard)
exterior_user_ids = tags.map{ |t| t.sub(/\Auser_/, '').to_i }
# which users have conversations on which shards?
users_by_conversation_shard =
ConversationParticipant.users_by_conversation_shard(exterior_user_ids)
# invert the map (to get shards-for-each-user rather than
# users-for-each-shard), then combine the keys (shards) according to mode.
# i.e. if we want conversations with all given users participating,
# intersect the set of shards; otherwise union them.
conversation_shards_by_user = {}
exterior_user_ids.each do |user_id|
conversation_shards_by_user[user_id] ||= Set.new
end
users_by_conversation_shard.each do |shard, user_ids|
user_ids.each do |user_id|
user_id = shard.relative_id_for(user_id)
conversation_shards_by_user[user_id] << shard
end
end
combinator = (options[:mode] == :or) ? :| : :&
conversation_shards =
conversation_shards_by_user.values.inject(combinator).to_a
# which conversations from those shards include any/all of the given users
# as participants?
conditions = Shard.with_each_shard(conversation_shards) do
user_ids = users_by_conversation_shard[Shard.current]
shard_conditions = if options[:mode] == :or || user_ids.size == 1
[<<-SQL, user_ids]
EXISTS (
SELECT *
FROM conversation_participants cp
WHERE cp.conversation_id = conversations.id
AND user_id IN (?)
)
SQL
else
[<<-SQL, user_ids, user_ids.size]
(
SELECT COUNT(*)
FROM conversation_participants cp
WHERE cp.conversation_id = conversations.id
AND user_id IN (?)
) = ?
SQL
end
# return arrays because with each shard is gonna try and Array() it
# anyways, and 1.8.7 would split up the multiline strings.
if Shard.current == scope_shard
[sanitize_sql(shard_conditions)]
else
conversation_ids = Conversation.where(shard_conditions).map do |c|
Shard.relative_id_for(c.id, scope_shard)
end
[sanitize_sql(:conversation_id => conversation_ids)]
end
end
# tagged will flatten a [single_condition] or [] into the list of
# conditions it's building up, but if we've got multiple conditions here,
# we want to make sure they're combined with OR regardless of
# options[:mode], since they're results per shard that we want to combine;
# each individual condition already takes options[:mode] into account)
if conditions.size > 1
"(#{conditions.join(' OR ')})"
else
conditions
end
sanitize_sql conditions
end
tagged_scope_handler(/\A(course|group|section)_(\d+)\z/) do |tags, options|
@ -376,7 +446,10 @@ class ConversationParticipant < ActiveRecord::Base
self.find(:all, :select => 'conversation_id', :order => order).map(&:conversation_id)
end
def self.users_by_conversation_shard(user_ids)
{ Shard.current => user_ids }
end
def update_one(update_params)
case update_params[:event]

3
lib/simple_tags.rb
View File

@ -25,6 +25,7 @@ module SimpleTags
tags.map{ |tag|
wildcard(quoted_table_name + '.tags', tag, :delimiter => ',')
}
conditions << sanitize_sql(['?', false]) if conditions.empty?
scoped({:conditions => conditions.join(options[:mode] == :or ? " OR " : " AND ")})
end
@ -86,4 +87,4 @@ module SimpleTags
klass.send :include, ReaderInstanceMethods
klass.send :include, WriterInstanceMethods
end
end
end

6
lib/stubs/shard.rb
View File

@ -109,6 +109,12 @@ ActiveRecord::Base.class_eval do
end
module ActiveRecord::Associations
AssociationProxy.class_eval do
def shard
Shard.default
end
end
%w{HasManyAssociation HasManyThroughAssociation}.each do |klass|
const_get(klass).class_eval do
def with_each_shard(*shards_or_options)

12
spec/apis/v1/conversations_api_spec.rb
View File

@ -253,6 +253,10 @@ describe ConversationsController, :type => :integration do
it "should recognize filter on the default shard" do
verify_filter(@alex.asset_string)
end
it "should recognize filter on an unrelated shard" do
@shard2.activate{ verify_filter(@alex.asset_string) }
end
end
context "tag user on non-default shard" do
@ -268,9 +272,17 @@ describe ConversationsController, :type => :integration do
@conversations << @shard1.activate{ conversation(@alex) }
end
it "should recognize filter on the default shard" do
verify_filter(@alex.asset_string)
end
it "should recognize filter on the user's shard" do
@shard1.activate{ verify_filter(@alex.asset_string) }
end
it "should recognize filter on an unrelated shard" do
@shard2.activate{ verify_filter(@alex.asset_string) }
end
end
end