diff --git a/app/jobs/cron/expired_dossiers_brouillon_deletion_job.rb b/app/jobs/cron/expired_dossiers_brouillon_deletion_job.rb new file mode 100644 index 000000000..5243db046 --- /dev/null +++ b/app/jobs/cron/expired_dossiers_brouillon_deletion_job.rb @@ -0,0 +1,7 @@ +class Cron::ExpiredDossiersBrouillonDeletionJob < Cron::CronJob + self.schedule_expression = "every day at 10 pm" + + def perform(*args) + ExpiredDossiersDeletionService.new.process_expired_dossiers_brouillon + end +end diff --git a/app/jobs/cron/expired_dossiers_deletion_job.rb b/app/jobs/cron/expired_dossiers_deletion_job.rb deleted file mode 100644 index 05ee53632..000000000 --- a/app/jobs/cron/expired_dossiers_deletion_job.rb +++ /dev/null @@ -1,9 +0,0 @@ -class Cron::ExpiredDossiersDeletionJob < Cron::CronJob - self.schedule_expression = "every day at 7 am" - - def perform(*args) - ExpiredDossiersDeletionService.process_expired_dossiers_brouillon - ExpiredDossiersDeletionService.process_expired_dossiers_en_construction - ExpiredDossiersDeletionService.process_expired_dossiers_termine - end -end diff --git a/app/jobs/cron/expired_dossiers_en_construction_deletion_job.rb b/app/jobs/cron/expired_dossiers_en_construction_deletion_job.rb new file mode 100644 index 000000000..84f5b3406 --- /dev/null +++ b/app/jobs/cron/expired_dossiers_en_construction_deletion_job.rb @@ -0,0 +1,7 @@ +class Cron::ExpiredDossiersEnConstructionDeletionJob < Cron::CronJob + self.schedule_expression = "every day at 3 pm" + + def perform(*args) + ExpiredDossiersDeletionService.new.process_expired_dossiers_en_construction + end +end diff --git a/app/jobs/cron/expired_dossiers_termine_deletion_job.rb b/app/jobs/cron/expired_dossiers_termine_deletion_job.rb new file mode 100644 index 000000000..ce47f3f5a --- /dev/null +++ b/app/jobs/cron/expired_dossiers_termine_deletion_job.rb @@ -0,0 +1,7 @@ +class Cron::ExpiredDossiersTermineDeletionJob < Cron::CronJob + self.schedule_expression = "every day at 7 am" + + def perform(*args) + ExpiredDossiersDeletionService.new.process_expired_dossiers_termine + end +end diff --git a/app/jobs/cron/weekly_overview_job.rb b/app/jobs/cron/weekly_overview_job.rb index f8857d759..45408c58d 100644 --- a/app/jobs/cron/weekly_overview_job.rb +++ b/app/jobs/cron/weekly_overview_job.rb @@ -1,16 +1,13 @@ class Cron::WeeklyOverviewJob < Cron::CronJob - self.schedule_expression = "every monday at 7 am" + self.schedule_expression = "every monday at 4 am" def perform # Feature flipped to avoid mails in staging due to unprocessed dossier return unless Rails.application.config.ds_weekly_overview Instructeur.find_each do |instructeur| - # NOTE: it's not exactly accurate because rate limit is not shared between jobs processes - Dolist::API.sleep_until_limit_reset if Dolist::API.near_rate_limit? - # mailer won't send anything if overview if empty - InstructeurMailer.last_week_overview(instructeur)&.deliver_later + InstructeurMailer.last_week_overview(instructeur)&.deliver_later(wait: rand(0..3.hours)) end end end diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index be55e4ab4..4577daf46 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -7,6 +7,8 @@ class ApplicationMailer < ActionMailer::Base default from: "#{APPLICATION_NAME} <#{CONTACT_EMAIL}>" layout 'mailer' + before_action -> { Sentry.set_tags(mailer: mailer_name, action: action_name) } + # Attach the procedure logo to the email (if any). # Returns the attachment url. def attach_logo(procedure) diff --git a/app/services/expired_dossiers_deletion_service.rb b/app/services/expired_dossiers_deletion_service.rb index a046ebd59..534a68f8f 100644 --- a/app/services/expired_dossiers_deletion_service.rb +++ b/app/services/expired_dossiers_deletion_service.rb @@ -1,20 +1,28 @@ class ExpiredDossiersDeletionService - def self.process_expired_dossiers_brouillon + def initialize(rate_limiter: MailRateLimiter.new(limit: 200, window: 10.minutes)) + @rate_limiter = rate_limiter + end + + def process_expired_dossiers_brouillon send_brouillon_expiration_notices delete_expired_brouillons_and_notify end - def self.process_expired_dossiers_en_construction + def process_expired_dossiers_en_construction send_en_construction_expiration_notices delete_expired_en_construction_and_notify end - def self.process_expired_dossiers_termine + def process_expired_dossiers_termine send_termine_expiration_notices delete_expired_termine_and_notify end - def self.send_brouillon_expiration_notices + def safe_send_email(mail) + @rate_limiter.send_with_delay(mail) + end + + def send_brouillon_expiration_notices dossiers_close_to_expiration = Dossier .brouillon_close_to_expiration .without_brouillon_expiration_notice_sent @@ -24,66 +32,70 @@ class ExpiredDossiersDeletionService dossiers_close_to_expiration.in_batches.update_all(brouillon_close_to_expiration_notice_sent_at: Time.zone.now) user_notifications.each do |(email, dossiers)| - DossierMailer.notify_brouillon_near_deletion( + mail = DossierMailer.notify_brouillon_near_deletion( dossiers, email - ).deliver_later + ) + safe_send_email(mail) end end - def self.send_en_construction_expiration_notices + def send_en_construction_expiration_notices send_expiration_notices( Dossier.en_construction_close_to_expiration.without_en_construction_expiration_notice_sent, :en_construction_close_to_expiration_notice_sent_at ) end - def self.send_termine_expiration_notices + def send_termine_expiration_notices send_expiration_notices( Dossier.termine_close_to_expiration.without_termine_expiration_notice_sent, :termine_close_to_expiration_notice_sent_at ) end - def self.delete_expired_brouillons_and_notify + def delete_expired_brouillons_and_notify user_notifications = group_by_user_email(Dossier.brouillon_expired) .map { |(email, dossiers)| [email, dossiers.map(&:hash_for_deletion_mail)] } Dossier.brouillon_expired.in_batches.destroy_all user_notifications.each do |(email, dossiers_hash)| - DossierMailer.notify_brouillon_deletion( + mail = DossierMailer.notify_brouillon_deletion( dossiers_hash, email - ).deliver_later + ) + safe_send_email(mail) end end - def self.delete_expired_en_construction_and_notify + def delete_expired_en_construction_and_notify delete_expired_and_notify(Dossier.en_construction_expired) end - def self.delete_expired_termine_and_notify + def delete_expired_termine_and_notify delete_expired_and_notify(Dossier.termine_expired, notify_on_closed_procedures_to_user: true) end private - def self.send_expiration_notices(dossiers_close_to_expiration, close_to_expiration_flag) + def send_expiration_notices(dossiers_close_to_expiration, close_to_expiration_flag) user_notifications = group_by_user_email(dossiers_close_to_expiration) administration_notifications = group_by_fonctionnaire_email(dossiers_close_to_expiration) dossiers_close_to_expiration.in_batches.update_all(close_to_expiration_flag => Time.zone.now) user_notifications.each do |(email, dossiers)| - DossierMailer.notify_near_deletion_to_user(dossiers, email).deliver_later + mail = DossierMailer.notify_near_deletion_to_user(dossiers, email) + safe_send_email(mail) end administration_notifications.each do |(email, dossiers)| - DossierMailer.notify_near_deletion_to_administration(dossiers, email).deliver_later + mail = DossierMailer.notify_near_deletion_to_administration(dossiers, email) + safe_send_email(mail) end end - def self.delete_expired_and_notify(dossiers_to_remove, notify_on_closed_procedures_to_user: false) + def delete_expired_and_notify(dossiers_to_remove, notify_on_closed_procedures_to_user: false) user_notifications = group_by_user_email(dossiers_to_remove, notify_on_closed_procedures_to_user: notify_on_closed_procedures_to_user) .map { |(email, dossiers)| [email, dossiers.map(&:id)] } administration_notifications = group_by_fonctionnaire_email(dossiers_to_remove) @@ -98,24 +110,26 @@ class ExpiredDossiersDeletionService user_notifications.each do |(email, dossier_ids)| dossier_ids = dossier_ids.intersection(deleted_dossier_ids) if dossier_ids.present? - DossierMailer.notify_automatic_deletion_to_user( + mail = DossierMailer.notify_automatic_deletion_to_user( DeletedDossier.where(dossier_id: dossier_ids).to_a, email - ).deliver_later + ) + safe_send_email(mail) end end administration_notifications.each do |(email, dossier_ids)| dossier_ids = dossier_ids.intersection(deleted_dossier_ids) if dossier_ids.present? - DossierMailer.notify_automatic_deletion_to_administration( + mail = DossierMailer.notify_automatic_deletion_to_administration( DeletedDossier.where(dossier_id: dossier_ids).to_a, email - ).deliver_later + ) + safe_send_email(mail) end end end - def self.group_by_user_email(dossiers, notify_on_closed_procedures_to_user: false) + def group_by_user_email(dossiers, notify_on_closed_procedures_to_user: false) dossiers .visible_by_user .with_notifiable_procedure(notify_on_closed: notify_on_closed_procedures_to_user) @@ -124,7 +138,7 @@ class ExpiredDossiersDeletionService .map { |(user, dossiers)| [user.email, dossiers] } end - def self.group_by_fonctionnaire_email(dossiers) + def group_by_fonctionnaire_email(dossiers) dossiers .visible_by_administration .with_notifiable_procedure(notify_on_closed: true) diff --git a/app/services/mail_rate_limiter.rb b/app/services/mail_rate_limiter.rb new file mode 100644 index 000000000..7e5a929b3 --- /dev/null +++ b/app/services/mail_rate_limiter.rb @@ -0,0 +1,32 @@ +class MailRateLimiter + attr_reader :delay, :current_window + + def send_with_delay(mail) + if current_window_full? + @delay += @window + end + if current_window_full? || current_window_expired? + @current_window = { started_at: Time.current, sent: 0 } + end + @current_window[:sent] += 1 + + mail.deliver_later(wait: delay) + end + + private + + def initialize(limit:, window:) + @limit = limit + @window = window + @current_window = { started_at: Time.current, sent: 0 } + @delay = 0 + end + + def current_window_expired? + (@current_window[:started_at] + @window).past? + end + + def current_window_full? + @current_window[:sent] >= @limit + end +end diff --git a/db/migrate/20230623160831_add_index_to_parent_dossier_id.rb b/db/migrate/20230623160831_add_index_to_parent_dossier_id.rb new file mode 100644 index 000000000..4610a9461 --- /dev/null +++ b/db/migrate/20230623160831_add_index_to_parent_dossier_id.rb @@ -0,0 +1,7 @@ +class AddIndexToParentDossierId < ActiveRecord::Migration[7.0] + disable_ddl_transaction! + + def change + add_index :dossiers, :parent_dossier_id, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index b3d1e30c2..a387e005c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_06_21_161733) do +ActiveRecord::Schema[7.0].define(version: 2023_06_23_160831) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -416,6 +416,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_06_21_161733) do t.index ["editing_fork_origin_id"], name: "index_dossiers_on_editing_fork_origin_id" t.index ["groupe_instructeur_id"], name: "index_dossiers_on_groupe_instructeur_id" t.index ["hidden_at"], name: "index_dossiers_on_hidden_at" + t.index ["parent_dossier_id"], name: "index_dossiers_on_parent_dossier_id" t.index ["prefill_token"], name: "index_dossiers_on_prefill_token", unique: true t.index ["revision_id"], name: "index_dossiers_on_revision_id" t.index ["state"], name: "index_dossiers_on_state" diff --git a/spec/lib/mail_rate_limiter_spec.rb b/spec/lib/mail_rate_limiter_spec.rb new file mode 100644 index 000000000..26418eafd --- /dev/null +++ b/spec/lib/mail_rate_limiter_spec.rb @@ -0,0 +1,26 @@ +describe MailRateLimiter do + describe 'hits limits' do + let(:limit) { 10 } + let(:window) { 2.seconds } + let(:rate_limiter) { MailRateLimiter.new(limit:, window:) } + let(:mail) { DossierMailer.notify_automatic_deletion_to_user([], 'tartampion@france.fr') } + + it 'decreases current_window[:limit]' do + expect { rate_limiter.send_with_delay(mail) }.to change { rate_limiter.current_window[:sent] }.by(1) + end + + it 'increases the delay by window when it reaches the max number of call' do + expect do + (limit + 1).times { rate_limiter.send_with_delay(mail) } + end.to change { rate_limiter.delay }.by(window) + end + + it 'renews current_window when it expires' do + rate_limiter.send_with_delay(mail) + travel_to(Time.current + window + 1.second) do + rate_limiter.send_with_delay(mail) + expect(rate_limiter.current_window[:sent]).to eq(1) + end + end + end +end diff --git a/spec/services/expired_dossiers_deletion_service_spec.rb b/spec/services/expired_dossiers_deletion_service_spec.rb index 653948ee1..767390bc5 100644 --- a/spec/services/expired_dossiers_deletion_service_spec.rb +++ b/spec/services/expired_dossiers_deletion_service_spec.rb @@ -6,7 +6,7 @@ describe ExpiredDossiersDeletionService do let(:procedure) { create(:procedure, :published, procedure_opts) } let(:procedure_2) { create(:procedure, :published, procedure_opts) } let(:reference_date) { Date.parse("March 8") } - + let(:service) { ExpiredDossiersDeletionService.new } describe '#process_expired_dossiers_brouillon' do before { Timecop.freeze(reference_date) } after { Timecop.return } @@ -26,7 +26,7 @@ describe ExpiredDossiersDeletionService do allow(DossierMailer).to receive(:notify_brouillon_near_deletion).and_call_original allow(DossierMailer).to receive(:notify_brouillon_deletion).and_call_original - ExpiredDossiersDeletionService.process_expired_dossiers_brouillon + service.process_expired_dossiers_brouillon end it 'emails should be sent' do @@ -58,7 +58,7 @@ describe ExpiredDossiersDeletionService do context 'with a single dossier' do let!(:dossier) { create(:dossier, procedure: procedure, created_at: created_at) } - before { ExpiredDossiersDeletionService.send_brouillon_expiration_notices } + before { service.send_brouillon_expiration_notices } context 'when the dossier is not close to expiration' do let(:created_at) { (conservation_par_defaut - 2.weeks - 1.day).ago } @@ -80,7 +80,7 @@ describe ExpiredDossiersDeletionService do let!(:dossier_1) { create(:dossier, procedure: procedure, user: user, created_at: (conservation_par_defaut - 2.weeks + 1.day).ago) } let!(:dossier_2) { create(:dossier, procedure: procedure_2, user: user, created_at: (conservation_par_defaut - 2.weeks + 1.day).ago) } - before { ExpiredDossiersDeletionService.send_brouillon_expiration_notices } + before { service.send_brouillon_expiration_notices } it { expect(DossierMailer).to have_received(:notify_brouillon_near_deletion).once } it { expect(DossierMailer).to have_received(:notify_brouillon_near_deletion).with(match_array([dossier_1, dossier_2]), user.email) } @@ -98,7 +98,7 @@ describe ExpiredDossiersDeletionService do context 'with a single dossier' do let!(:dossier) { create(:dossier, procedure: procedure, brouillon_close_to_expiration_notice_sent_at: notice_sent_at) } - before { ExpiredDossiersDeletionService.delete_expired_brouillons_and_notify } + before { service.delete_expired_brouillons_and_notify } context 'when no notice has been sent' do let(:notice_sent_at) { nil } @@ -128,7 +128,7 @@ describe ExpiredDossiersDeletionService do let!(:dossier_1) { create(:dossier, procedure: procedure, user: user, brouillon_close_to_expiration_notice_sent_at: (warning_period + 1.day).ago) } let!(:dossier_2) { create(:dossier, procedure: procedure_2, user: user, brouillon_close_to_expiration_notice_sent_at: (warning_period + 1.day).ago) } - before { ExpiredDossiersDeletionService.delete_expired_brouillons_and_notify } + before { service.delete_expired_brouillons_and_notify } it { expect(DossierMailer).to have_received(:notify_brouillon_deletion).once } it { expect(DossierMailer).to have_received(:notify_brouillon_deletion).with(match_array([dossier_1.hash_for_deletion_mail, dossier_2.hash_for_deletion_mail]), user.email) } @@ -147,7 +147,7 @@ describe ExpiredDossiersDeletionService do context 'with a single dossier' do let!(:dossier) { create(:dossier, :en_construction, :followed, procedure: procedure, en_construction_at: en_construction_at) } - before { ExpiredDossiersDeletionService.send_en_construction_expiration_notices } + before { service.send_en_construction_expiration_notices } context 'when the dossier is not near deletion' do let(:en_construction_at) { (conservation_par_defaut - 2.weeks - 1.day).ago } @@ -178,7 +178,7 @@ describe ExpiredDossiersDeletionService do before do instructeur.followed_dossiers << dossier_1 << dossier_2 - ExpiredDossiersDeletionService.send_en_construction_expiration_notices + service.send_en_construction_expiration_notices end it { expect(DossierMailer).to have_received(:notify_near_deletion_to_user).once } @@ -195,7 +195,7 @@ describe ExpiredDossiersDeletionService do before do administrateur.instructeur.followed_dossiers << dossier - ExpiredDossiersDeletionService.send_en_construction_expiration_notices + service.send_en_construction_expiration_notices end it { expect(DossierMailer).to have_received(:notify_near_deletion_to_user).once } @@ -219,7 +219,7 @@ describe ExpiredDossiersDeletionService do let!(:dossier) { create(:dossier, :en_construction, :followed, procedure: procedure, en_construction_close_to_expiration_notice_sent_at: notice_sent_at) } let(:deleted_dossier) { DeletedDossier.find_by(dossier_id: dossier.id) } - before { ExpiredDossiersDeletionService.delete_expired_en_construction_and_notify } + before { service.delete_expired_en_construction_and_notify } context 'when no notice has been sent' do let(:notice_sent_at) { nil } @@ -261,7 +261,7 @@ describe ExpiredDossiersDeletionService do before do instructeur.followed_dossiers << dossier_1 << dossier_2 - ExpiredDossiersDeletionService.delete_expired_en_construction_and_notify + service.delete_expired_en_construction_and_notify end it { expect(DossierMailer).to have_received(:notify_automatic_deletion_to_user).once } @@ -290,7 +290,7 @@ describe ExpiredDossiersDeletionService do context 'with a single dossier' do let!(:dossier) { create(:dossier, :followed, state: :accepte, procedure: procedure, processed_at: processed_at) } - before { ExpiredDossiersDeletionService.send_termine_expiration_notices } + before { service.send_termine_expiration_notices } context 'when the dossier is not near deletion' do let(:processed_at) { (conservation_par_defaut - 2.weeks - 1.day).ago } @@ -321,7 +321,7 @@ describe ExpiredDossiersDeletionService do before do instructeur.followed_dossiers << dossier_1 << dossier_2 - ExpiredDossiersDeletionService.send_termine_expiration_notices + service.send_termine_expiration_notices end it { expect(DossierMailer).to have_received(:notify_near_deletion_to_user).once } @@ -338,7 +338,7 @@ describe ExpiredDossiersDeletionService do before do administrateur.instructeur.followed_dossiers << dossier - ExpiredDossiersDeletionService.send_termine_expiration_notices + service.send_termine_expiration_notices end it { expect(DossierMailer).to have_received(:notify_near_deletion_to_user).once } @@ -366,7 +366,7 @@ describe ExpiredDossiersDeletionService do let!(:dossier) { create(:dossier, :followed, :accepte, procedure: procedure, termine_close_to_expiration_notice_sent_at: notice_sent_at) } let(:deleted_dossier) { DeletedDossier.find_by(dossier_id: dossier.id) } - before { ExpiredDossiersDeletionService.delete_expired_termine_and_notify } + before { service.delete_expired_termine_and_notify } context 'when no notice has been sent' do let(:notice_sent_at) { nil } @@ -408,7 +408,7 @@ describe ExpiredDossiersDeletionService do before do instructeur.followed_dossiers << dossier_1 << dossier_2 - ExpiredDossiersDeletionService.delete_expired_termine_and_notify + service.delete_expired_termine_and_notify end it { expect(DossierMailer).to have_received(:notify_automatic_deletion_to_user).once } @@ -430,7 +430,7 @@ describe ExpiredDossiersDeletionService do before do instructeur.followed_dossiers << dossier_1 << dossier_2 - ExpiredDossiersDeletionService.delete_expired_termine_and_notify + service.delete_expired_termine_and_notify end it { expect(DossierMailer).to have_received(:notify_automatic_deletion_to_user).once }