make notification endpoints multi-shard

test plan
 - have user on shard1 with push notifications
 - merge into user on shard2
 - push should work
 - deleting push channel should work

fixes VICE-790
flag=none

Change-Id: Ib98b907017bb8d5dcee18d0346c4240d8381cfaf
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/247468
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>

app/controllers/communication_channels_controller.rb

@@ -165,7 +165,7 @@ class CommunicationChannelsController < ApplicationController
       end
 
       NotificationEndpoint.unique_constraint_retry do
-        unless @current_user.notification_endpoints.where("lower(token) = lower(?)", params[:communication_channel][:token]).exists?
+        unless @access_token.notification_endpoints.where("lower(token) = lower(?)", params[:communication_channel][:token]).exists?
           @access_token.notification_endpoints.create!(token: params[:communication_channel][:token])
         end
       end
@@ -530,7 +530,7 @@ class CommunicationChannelsController < ApplicationController
     @cc = @current_user.communication_channels.unretired.of_type(CommunicationChannel::TYPE_PUSH).take
     raise ActiveRecord::RecordNotFound unless @cc
 
-    endpoints = @current_user.notification_endpoints.where("lower(token) = ?", params[:push_token].downcase)
+    endpoints = @current_user.notification_endpoints.shard(@current_user).where("lower(token) = ?", params[:push_token].downcase)
     if endpoints&.destroy_all
       @current_user.touch
       return render json: {success: true}

app/models/user.rb

@@ -106,9 +106,9 @@ class User < ActiveRecord::Base
   has_many :associated_root_accounts, -> { order("user_account_associations.depth").
     where(accounts: { parent_account_id: nil }) }, source: :account, through: :user_account_associations
   has_many :developer_keys
-  has_many :access_tokens, -> { where(:workflow_state => "active").preload(:developer_key) }, inverse_of: :user
+  has_many :access_tokens, -> { where(:workflow_state => "active").preload(:developer_key) }, inverse_of: :user, multishard: true
   has_many :masquerade_tokens, -> { where(:workflow_state => "active").preload(:developer_key) }, class_name: 'AccessToken', inverse_of: :real_user
-  has_many :notification_endpoints, :through => :access_tokens
+  has_many :notification_endpoints, :through => :access_tokens, multishard: true
   has_many :context_external_tools, -> { order(:name) }, as: :context, inverse_of: :context, dependent: :destroy
   has_many :lti_results, inverse_of: :user, class_name: 'Lti::Result', dependent: :destroy
 

spec/apis/v1/communication_channels_api_spec.rb

@@ -150,6 +150,7 @@ describe 'CommunicationChannels API', type: :request do
     it "doesn't error if the user already has a login with the same e-mail address" do
       @someone.pseudonyms.create!(unique_id: 'new+api@example.com')
       api_call(:post, @path, @path_options, @post_params.merge(skip_confirmation: 1))
+      expect(response).to be_successful
     end
 
     context 'a site admin' do
@@ -217,21 +218,28 @@ describe 'CommunicationChannels API', type: :request do
         expect(response.code).to eql '401'
       end
 
-      context 'push' do
-        before { @post_params.merge!(communication_channel: {token: 'registration_token', type: 'push'}) }
-
+      context 'not configured push' do
         it 'should complain about sns not being configured' do
+          @post_params.merge!(communication_channel: {token: 'registration_token', type: 'push'})
           raw_api_call(:post, @path, @path_options, @post_params)
 
           expect(response.code).to eql '400'
         end
+      end
 
-        it "should work" do
-          client = double()
-          allow(DeveloperKey).to receive(:sns).and_return(client)
+      context 'push' do
+        before { @post_params.merge!(communication_channel: {token: 'registration_token', type: 'push'}) }
+
+        let(:client) { double() }
+        let(:dk) do
           dk = DeveloperKey.default
           dk.sns_arn = 'apparn'
           dk.save!
+          dk
+        end
+
+        it "should work" do
+          allow(DeveloperKey).to receive(:sns).and_return(client)
           $spec_api_tokens[@user] = @user.access_tokens.create!(developer_key: dk).full_token
           expect(client).to receive(:create_platform_endpoint).once.and_return(endpoint_arn: 'endpointarn')
 
@@ -242,11 +250,7 @@ describe 'CommunicationChannels API', type: :request do
         end
 
         it "shouldn't create two push channels regardless of case" do
-          client = double()
           allow(DeveloperKey).to receive(:sns).and_return(client)
-          dk = DeveloperKey.default
-          dk.sns_arn = 'apparn'
-          dk.save!
           $spec_api_tokens[@user] = @user.access_tokens.create!(developer_key: dk).full_token
           expect(client).to receive(:create_platform_endpoint).once.and_return(endpoint_arn: 'endpointarn')
           @post_params[:communication_channel][:token].upcase!
@@ -255,6 +259,27 @@ describe 'CommunicationChannels API', type: :request do
           api_call(:post, @path, @path_options, @post_params)
           expect(@user.notification_endpoints.count).to eq 1
         end
+
+        context 'shards' do
+          specs_require_sharding
+
+          it "should not have unique constraint error for push channel" do
+            allow(DeveloperKey).to receive(:sns).and_return(client)
+            $spec_api_tokens[@user] = @user.access_tokens.create!(developer_key: dk).full_token
+            expect(client).to receive(:create_platform_endpoint).once.and_return(endpoint_arn: 'endpointarn')
+            api_call(:post, @path, @path_options, @post_params)
+            @shard1.activate { @new_user = User.create!(name: 'shard one') }
+            # this is faster than a user_merge
+            @new_user.associate_with_shard(Shard.current)
+            @user.access_tokens.update_all(user_id: @new_user.id)
+            $spec_api_tokens[@new_user] = $spec_api_tokens[@user]
+            @user = @new_user
+            @path_options[:user_id] = @user.id
+            api_call(:post, "/api/v1/users/#{@user.id}/communication_channels", @path_options, @post_params)
+            expect(response).to be_successful
+          end
+        end
+
       end
     end
   end

spec/controllers/communication_channels_controller_spec.rb

@@ -1387,6 +1387,22 @@ describe CommunicationChannelsController do
         let(:fake_token) { 'insttothemoon' }
         before(:each) { sns_access_token.notification_endpoints.create!(token: fake_token) }
 
+        context "cross-shard user" do
+          specs_require_sharding
+
+          it 'should delete endpoints from all_shards', type: :request do
+            @shard1.activate { @new_user = User.create!(name: 'shard one') }
+            UserMerge.from(@user).into(@new_user)
+            @user = @new_user
+            json = api_call(:delete, "/api/v1/users/self/communication_channels/push",
+                            {controller: 'communication_channels', action: 'delete_push_token', format: 'json',
+                             push_token: fake_token}, {push_token: fake_token})
+            expect(json['success']).to eq true
+            endpoints = @user.notification_endpoints.shard(@user).where("lower(token) = ?", fake_token)
+            expect(endpoints.length).to eq 0
+          end
+        end
+
         it 'should delete a push_token', type: :request do
           json = api_call(:delete, "/api/v1/users/self/communication_channels/push",
                           {controller: 'communication_channels', action: 'delete_push_token', format: 'json',