From de740dfb9cb55af5d77ebdcea84133521200de73 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 26 Mar 2024 16:57:08 +0100 Subject: [PATCH] Use `upsert_all` and `insert_all` to reduce back-and-forth in costly migrations (#29752) --- ..._migrate_interaction_settings_to_policy.rb | 70 +++++++++-------- ...te_interaction_settings_to_policy_again.rb | 76 +++++++++---------- 2 files changed, 75 insertions(+), 71 deletions(-) diff --git a/db/migrate/20240304090449_migrate_interaction_settings_to_policy.rb b/db/migrate/20240304090449_migrate_interaction_settings_to_policy.rb index a0ce0a663..ea4cfccdf 100644 --- a/db/migrate/20240304090449_migrate_interaction_settings_to_policy.rb +++ b/db/migrate/20240304090449_migrate_interaction_settings_to_policy.rb @@ -4,46 +4,50 @@ class MigrateInteractionSettingsToPolicy < ActiveRecord::Migration[7.1] disable_ddl_transaction! # Dummy classes, to make migration possible across version changes - class Account < ApplicationRecord - has_one :user, inverse_of: :account - has_one :notification_policy, inverse_of: :account - end - class User < ApplicationRecord - belongs_to :account + belongs_to :notification_policy, foreign_key: 'account_id', primary_key: 'account_id', optional: true, inverse_of: false end - class NotificationPolicy < ApplicationRecord - belongs_to :account - end + class NotificationPolicy < ApplicationRecord; end def up - User.includes(account: :notification_policy).find_each do |user| - deserialized_settings = Oj.load(user.attributes_before_type_cast['settings']) - - next if deserialized_settings.nil? - - policy = user.account.notification_policy || user.account.build_notification_policy - requires_new_policy = false - - if deserialized_settings['interactions.must_be_follower'] - policy.filter_not_followers = true - requires_new_policy = true - end - - if deserialized_settings['interactions.must_be_following'] - policy.filter_not_following = true - requires_new_policy = true - end - - unless deserialized_settings['interactions.must_be_following_dm'] - policy.filter_private_mentions = false - requires_new_policy = true - end - - policy.save if requires_new_policy && policy.changed? + User.includes(:notification_policy).in_batches do |users| + NotificationPolicy.upsert_all(users.filter_map { |user| policy_for_user(user) }) end end def down; end + + private + + def policy_for_user(user) + deserialized_settings = Oj.load(user.attributes_before_type_cast['settings']) + return if deserialized_settings.nil? + + requires_new_policy = false + + policy = { + account_id: user.account_id, + filter_not_followers: false, + filter_not_following: false, + filter_private_mentions: true, + } + + if deserialized_settings['interactions.must_be_follower'] + policy[:filter_not_followers] = true + requires_new_policy = true + end + + if deserialized_settings['interactions.must_be_following'] + policy[:filter_not_following] = true + requires_new_policy = true + end + + unless deserialized_settings['interactions.must_be_following_dm'] + policy[:filter_private_mentions] = false + requires_new_policy = true + end + + policy if requires_new_policy + end end diff --git a/db/post_migrate/20240321160706_migrate_interaction_settings_to_policy_again.rb b/db/post_migrate/20240321160706_migrate_interaction_settings_to_policy_again.rb index 9baefa677..c789b6395 100644 --- a/db/post_migrate/20240321160706_migrate_interaction_settings_to_policy_again.rb +++ b/db/post_migrate/20240321160706_migrate_interaction_settings_to_policy_again.rb @@ -4,51 +4,51 @@ class MigrateInteractionSettingsToPolicyAgain < ActiveRecord::Migration[7.1] disable_ddl_transaction! # Dummy classes, to make migration possible across version changes - class Account < ApplicationRecord - has_one :user, inverse_of: :account - has_one :notification_policy, inverse_of: :account - end - class User < ApplicationRecord - belongs_to :account + belongs_to :notification_policy, foreign_key: 'account_id', primary_key: 'account_id', optional: true, inverse_of: false end - class NotificationPolicy < ApplicationRecord - belongs_to :account - end + class NotificationPolicy < ApplicationRecord; end def up - User.includes(account: :notification_policy).find_each do |user| - deserialized_settings = Oj.load(user.attributes_before_type_cast['settings']) - - next if deserialized_settings.nil? - - # If the user has configured a notification policy, don't override it - next if user.account.notification_policy.present? - - policy = user.account.build_notification_policy - requires_new_policy = false - - if deserialized_settings['interactions.must_be_follower'] - policy.filter_not_followers = true - requires_new_policy = true - end - - if deserialized_settings['interactions.must_be_following'] - policy.filter_not_following = true - requires_new_policy = true - end - - unless deserialized_settings['interactions.must_be_following_dm'] - policy.filter_private_mentions = false - requires_new_policy = true - end - - policy.save if requires_new_policy && policy.changed? - rescue ActiveRecord::RecordNotUnique - next + User.includes(:notification_policy).in_batches do |users| + NotificationPolicy.insert_all(users.filter_map { |user| policy_for_user(user) }) end end def down; end + + private + + def policy_for_user(user) + deserialized_settings = Oj.load(user.attributes_before_type_cast['settings']) + return if deserialized_settings.nil? + return if user.notification_policy.present? + + requires_new_policy = false + + policy = { + account_id: user.account_id, + filter_not_followers: false, + filter_not_following: false, + filter_private_mentions: true, + } + + if deserialized_settings['interactions.must_be_follower'] + policy[:filter_not_followers] = true + requires_new_policy = true + end + + if deserialized_settings['interactions.must_be_following'] + policy[:filter_not_following] = true + requires_new_policy = true + end + + unless deserialized_settings['interactions.must_be_following_dm'] + policy[:filter_private_mentions] = false + requires_new_policy = true + end + + policy if requires_new_policy + end end