From 9fcfff58a4bd6e17ad3dc13fb93dd897aaac0d8d Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Thu, 7 Sep 2023 19:49:53 +0100 Subject: [PATCH] Add ability to ask external caches to invalidate. Add a cached_urls method to the Comment, FoiAttachment, InfoRequest, PublicBody and User models, containing the entries that those models wish to invalidate when an event is logged via log_event or when the censor rules change. Entries are either a fixed path to be purged or a regex beginning with '^' to be banned. Adds a NotifyCacheJob that for each of the object's cached_urls, for each locale, for each host in VARNISH_HOSTS, makes BAN or PURGE HTTP requests to the host, to invalidate the relevant entries. --- app/jobs/notify_cache_job.rb | 55 +++++++++++++++++ app/models/censor_rule.rb | 3 + app/models/comment.rb | 7 +++ app/models/foi_attachment.rb | 8 +++ app/models/info_request.rb | 38 ++++++++++++ app/models/public_body.rb | 17 +++++- app/models/user.rb | 17 +++++- config/general.yml-example | 14 +++++ lib/configuration.rb | 1 + .../admin/foi_attachments_controller_spec.rb | 3 + .../admin_incoming_message_controller_spec.rb | 7 +++ spec/jobs/notify_cache_job_spec.rb | 61 +++++++++++++++++++ spec/models/censor_rule_spec.rb | 5 +- spec/models/comment_spec.rb | 12 ++++ spec/models/foi_attachment_spec.rb | 10 +++ spec/models/info_request_spec.rb | 20 ++++++ spec/models/public_body_spec.rb | 8 +++ spec/models/user_spec.rb | 7 +++ 18 files changed, 290 insertions(+), 3 deletions(-) create mode 100644 app/jobs/notify_cache_job.rb create mode 100644 spec/jobs/notify_cache_job_spec.rb diff --git a/app/jobs/notify_cache_job.rb b/app/jobs/notify_cache_job.rb new file mode 100644 index 00000000000..fc69953eee2 --- /dev/null +++ b/app/jobs/notify_cache_job.rb @@ -0,0 +1,55 @@ +require 'net/http' + +class Net::HTTP::Purge < Net::HTTP::Get + METHOD = 'PURGE' +end + +class Net::HTTP::Ban < Net::HTTP::Get + METHOD = 'BAN' +end + +## +# Job to notify a cache of URLs to be purged or banned, given an object +# (that must have a cached_urls method). +# +# Examples: +# NotifyCacheJob.perform(InfoRequest.first) +# NotifyCacheJob.perform(FoiAttachment.first) +# NotifyCacheJob.perform(Comment.first) +# +class NotifyCacheJob < ApplicationJob + queue_as :default + + def perform(object) + urls = object.cached_urls + locales = [''] + AlaveteliLocalization.available_locales.map { |locale| + '/' + locale + } + hosts = AlaveteliConfiguration.varnish_hosts + locales.each do |locale| + hosts.each do |host| + Net::HTTP.start(AlaveteliConfiguration.domain, 80, host, 6081) do |http| + urls.each do |url| + if url.start_with? '^' + request = Net::HTTP::Ban.new('/') + request['X-Invalidate-Pattern'] = '^' + locale + url[1..-1] + else + request = Net::HTTP::Purge.new(locale + url) + end + response = http.request(request) + result = response.code + if result == "200" + Rails.logger.debug( + "PURGE: Purged URL #{url} at #{host}: #{result}" + ) + else + Rails.logger.warn( + "PURGE: Unable to purge URL #{url} at #{host}: status #{result}" + ) + end + end + end + end + end + end +end diff --git a/app/models/censor_rule.rb b/app/models/censor_rule.rb index a3704e9b6c2..c941236672d 100644 --- a/app/models/censor_rule.rb +++ b/app/models/censor_rule.rb @@ -75,6 +75,9 @@ def is_global? def expire_requests if info_request InfoRequestExpireJob.perform_later(info_request) + if AlaveteliConfiguration.varnish_hosts.present? + NotifyCacheJob.perform_later(info_request) + end elsif user InfoRequestExpireJob.perform_later(user, :info_requests) elsif public_body diff --git a/app/models/comment.rb b/app/models/comment.rb index 07b295d2713..747a66a7df6 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -193,6 +193,13 @@ def hide(editor:) end end + def cached_urls + [ + request_path(info_request), + show_user_wall_path(url_name: user.url_name) + ] + end + private def check_body_has_content diff --git a/app/models/foi_attachment.rb b/app/models/foi_attachment.rb index 0ed0ae02293..104f21c5d36 100644 --- a/app/models/foi_attachment.rb +++ b/app/models/foi_attachment.rb @@ -29,6 +29,8 @@ require 'digest' class FoiAttachment < ApplicationRecord + include Rails.application.routes.url_helpers + include LinkToHelper include MessageProminence MissingAttachment = Class.new(StandardError) @@ -318,6 +320,12 @@ def body_as_html(dir, opts = {}) AttachmentToHTML.to_html(self, to_html_opts) end + def cached_urls + [ + request_path(incoming_message.info_request) + ] + end + private def text_type? diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 73b3b1039e3..38684684d5b 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -52,6 +52,7 @@ class InfoRequest < ApplicationRecord include InfoRequest::TitleValidation include Taggable include Notable + include LinkToHelper admin_columns exclude: %i[title url_title], include: %i[rejected_incoming_count] @@ -1163,6 +1164,15 @@ def log_event(type, params, options = {}) if !last_event_time || (event.created_at > last_event_time) update_column(:last_event_time, event.created_at) end + if AlaveteliConfiguration.varnish_hosts.present? + if %w[comment edit_comment hide_comment].include? type + NotifyCacheJob.perform_later(Comment.find(params[:comment_id])) + elsif type == 'edit_attachment' + NotifyCacheJob.perform_later(FoiAttachment.find(params[:attachment_id])) + else + NotifyCacheJob.perform_later(self) + end + end event end @@ -1758,6 +1768,34 @@ def latest_refusals incoming_messages.select(&:refusals?).last&.refusals || [] end + def cached_urls + feed_request = TrackThing.new( + info_request: self, + track_type: 'request_updates' + ) + feed_body = TrackThing.new( + public_body: public_body, + track_type: 'public_body_updates' + ) + feed_user = TrackThing.new( + tracked_user: user, + track_type: 'user_updates' + ) + [ + '/', + public_body_path(public_body), + request_path(self), + request_details_path(self), + '^/list', + do_track_path(feed_request, feed = 'feed'), + '^/feed/list/', + do_track_path(feed_body, feed = 'feed'), + do_track_path(feed_user, feed = 'feed'), + user_path(user), + show_user_wall_path(url_name: user.url_name) + ] + end + private def self.add_conditions_from_extra_params(params, extra_params) diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 65ef2c552c5..48e3f8a2f14 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -35,6 +35,8 @@ class PublicBody < ApplicationRecord include Taggable include Notable + include Rails.application.routes.url_helpers + include LinkToHelper class ImportCSVDryRun < StandardError; end @@ -114,7 +116,7 @@ def self.admin_title after_save :update_missing_email_tag - after_update :reindex_requested_from + after_update :reindex_requested_from, :invalidate_cached_pages # Every public body except for the internal admin one is visible scope :visible, -> { where("public_bodies.id <> #{ PublicBody.internal_admin_body.id }") } @@ -911,6 +913,14 @@ def questions PublicBodyQuestion.fetch(self) end + def cached_urls + [ + public_body_path(self), + list_public_bodies_path, + '^/body/list' + ] + end + private # If the url_name has changed, then all requested_from: queries will break @@ -919,6 +929,11 @@ def reindex_requested_from expire_requests if saved_change_to_attribute?(:url_name) end + def invalidate_cached_pages + return unless AlaveteliConfiguration.varnish_hosts.present? + NotifyCacheJob.perform_later(self) + end + # Read an attribute value (without using locale fallbacks if the # attribute is translated) def read_attribute_value(name, locale) diff --git a/app/models/user.rb b/app/models/user.rb index c33aae63d4b..7d8239a4ce5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -47,6 +47,8 @@ class User < ApplicationRecord include User::OneTimePassword include User::Slug include User::Survey + include Rails.application.routes.url_helpers + include LinkToHelper DEFAULT_CONTENT_LIMITS = { info_requests: AlaveteliConfiguration.max_requests_per_user_per_day, @@ -179,7 +181,9 @@ class User < ApplicationRecord validate :email_and_name_are_valid after_initialize :set_defaults - after_update :reindex_referencing_models, :update_pro_account + after_update :reindex_referencing_models, + :update_pro_account, + :invalidate_cached_pages acts_as_xapian texts: [:name, :about_me], values: [ @@ -341,6 +345,11 @@ def expire_comments comments.find_each(&:reindex_request_events) end + def invalidate_cached_pages + return unless AlaveteliConfiguration.varnish_hosts.present? + NotifyCacheJob.perform_later(self) + end + def locale (super || AlaveteliLocalization.locale).to_s end @@ -673,6 +682,12 @@ def flipper_id "User;#{id}" end + def cached_urls + [ + user_path(self) + ] + end + private def set_defaults diff --git a/config/general.yml-example b/config/general.yml-example index f577b0c71cc..89af7a927cf 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -627,6 +627,20 @@ EXCEPTION_NOTIFICATIONS_TO: # --- MAX_REQUESTS_PER_USER_PER_DAY: 6 +# If you're running behind Varnish set this to work out where to send purge +# requests. Otherwise, don't set it. +# +# VARNISH_HOSTS - Array of Strings (default: nil) +# +# Examples: +# +# VARNISH_HOSTS: +# - host1 +# - host2 +# +# --- +VARNISH_HOSTS: null + # Adding a value here will enable Google Analytics on all non-admin pages for # non-admin users. # diff --git a/lib/configuration.rb b/lib/configuration.rb index ad1d2845db7..926da9169ae 100644 --- a/lib/configuration.rb +++ b/lib/configuration.rb @@ -131,6 +131,7 @@ module AlaveteliConfiguration USE_MAILCATCHER_IN_DEVELOPMENT: true, USER_SIGN_IN_ACTIVITY_RETENTION_DAYS: 0, UTILITY_SEARCH_PATH: ['/usr/bin', '/usr/local/bin'], + VARNISH_HOSTS: [], WORKING_OR_CALENDAR_DAYS: 'working' } # rubocop:enable Layout/LineLength diff --git a/spec/controllers/admin/foi_attachments_controller_spec.rb b/spec/controllers/admin/foi_attachments_controller_spec.rb index d37c2a5437e..7f37d4ed093 100644 --- a/spec/controllers/admin/foi_attachments_controller_spec.rb +++ b/spec/controllers/admin/foi_attachments_controller_spec.rb @@ -78,6 +78,9 @@ end it 'should log an "edit_attachment" event on the info_request' do + allow(AlaveteliConfiguration).to receive(:varnish_hosts). + and_return(['varnish']) + expect(NotifyCacheJob).to receive(:perform_later).with(attachment) allow(@controller).to receive(:admin_current_user). and_return("Admin user") diff --git a/spec/controllers/admin_incoming_message_controller_spec.rb b/spec/controllers/admin_incoming_message_controller_spec.rb index b2557d66631..0450068116f 100644 --- a/spec/controllers/admin_incoming_message_controller_spec.rb +++ b/spec/controllers/admin_incoming_message_controller_spec.rb @@ -17,6 +17,9 @@ end it "destroys the ActiveStorage attachment record" do + allow(AlaveteliConfiguration).to receive(:varnish_hosts). + and_return(['varnish']) + expect(NotifyCacheJob).to receive(:perform_later).with(@im.info_request) file = @im.raw_email.file expect(file.attached?).to eq true post :destroy, params: { id: @im.id } @@ -217,6 +220,10 @@ def make_request(params=@default_params) it 'should log an "edit_incoming" event on the info_request' do allow(@controller).to receive(:admin_current_user).and_return("Admin user") + allow(AlaveteliConfiguration).to receive(:varnish_hosts). + and_return(['varnish']) + expect(NotifyCacheJob).to receive(:perform_later). + with(@incoming.info_request) make_request @incoming.reload last_event = @incoming.info_request_events.last diff --git a/spec/jobs/notify_cache_job_spec.rb b/spec/jobs/notify_cache_job_spec.rb new file mode 100644 index 00000000000..abe18d7c4b3 --- /dev/null +++ b/spec/jobs/notify_cache_job_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' + +RSpec.describe NotifyCacheJob, type: :job do + let(:args) { [] } + subject(:perform) { described_class.new.perform(*args) } + + let(:request) { FactoryBot.build(:info_request) } + let(:comment) { FactoryBot.build(:comment) } + let(:attachment) { FactoryBot.build(:pdf_attachment) } + let(:im) { FactoryBot.create(:plain_incoming_message) } + + before do + @old_include_default_locale_in_urls = + AlaveteliConfiguration.include_default_locale_in_urls + AlaveteliLocalization.set_default_locale_urls(false) + + allow(AlaveteliConfiguration).to receive(:varnish_hosts). + and_return(['varnish']) + + stub_request(:purge, /^http:\/\/test\.host(\/(en|es|fr|en_GB))?\/(|((feed\/)?body|(feed\/|details\/)?request|(feed\/)?user)\/[a-z0-9_]+|user\/[a-z0-9_]+\/wall)$/). + to_return(status: 200, body: "", headers: {}) + stub_request(:ban, 'http://test.host/'). + with(headers: + { + 'X-Invalidate-Pattern' => + /^\^(\/(en|es|fr|en_GB))?\/(list|feed\/list\/)$/ + }). + to_return(status: 200, body: "", headers: {}) + end + + after do + AlaveteliLocalization.set_default_locale_urls( + @old_include_default_locale_in_urls + ) + end + + context 'when called with a request' do + let(:args) { [request] } + it 'calls out to varnish correctly' do + expect(Rails.logger).to receive(:debug).exactly(55).times + perform + end + end + + context 'when called with a comment' do + let(:args) { [comment] } + it 'calls out to varnish correctly' do + expect(Rails.logger).to receive(:debug).exactly(10).times + perform + end + end + + context 'when called with an attachment' do + let(:args) { [attachment] } + it 'calls out to varnish correctly' do + attachment.incoming_message = im + expect(Rails.logger).to receive(:debug).exactly(5).times + perform + end + end +end diff --git a/spec/models/censor_rule_spec.rb b/spec/models/censor_rule_spec.rb index 7ca697f288c..d1a55acf720 100644 --- a/spec/models/censor_rule_spec.rb +++ b/spec/models/censor_rule_spec.rb @@ -159,12 +159,15 @@ end describe '#expire_requests' do - it 'create expire job for the request if it is a request rule' do + allow(AlaveteliConfiguration).to receive(:varnish_hosts). + and_return(['varnish']) + request = FactoryBot.create(:info_request) rule = FactoryBot.create(:info_request_censor_rule, info_request: request) expect(InfoRequestExpireJob).to receive(:perform_later).with(request) + expect(NotifyCacheJob).to receive(:perform_later).with(request) rule.expire_requests end diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 608793c437c..fcf6ae67167 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -188,6 +188,15 @@ end # rubocop:enable Layout/FirstArrayElementIndentation + describe '.cached_urls' do + it 'includes the correct paths' do + comment = FactoryBot.create(:comment) + request_path = "/request/" + comment.info_request.url_title + user_wall_path = "/user/" + comment.user.url_name + "/wall" + expect(comment.cached_urls).to eq([request_path, user_wall_path]) + end + end + describe '#prominence' do subject { comment.prominence } @@ -344,6 +353,9 @@ end it 'logs an event on the request' do + allow(AlaveteliConfiguration).to receive(:varnish_hosts). + and_return(['varnish']) + expect(NotifyCacheJob).to receive(:perform_later).with(comment) subject event = comment.info_request.last_event expect(event.event_type).to eq('hide_comment') diff --git a/spec/models/foi_attachment_spec.rb b/spec/models/foi_attachment_spec.rb index 14a21f51978..cf0f31dc8e0 100644 --- a/spec/models/foi_attachment_spec.rb +++ b/spec/models/foi_attachment_spec.rb @@ -43,6 +43,16 @@ it { is_expected.to match_array(binary_attachments) } end + describe '.cached_urls' do + it 'includes the correct paths' do + att = FactoryBot.create(:jpeg_attachment) + im = FactoryBot.create(:plain_incoming_message) + att.incoming_message = im + request_path = "/request/" + att.incoming_message.info_request.url_title + expect(att.cached_urls).to eq([request_path]) + end + end + describe '#body=' do it "sets the body" do diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 5ae655fafd3..b5ab265ac73 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -108,6 +108,26 @@ it { is_expected.to eq(4) } end + describe '.cached_urls' do + it 'includes the correct paths' do + request = FactoryBot.create(:info_request) + expect(request.cached_urls). + to eq([ + '/', + '/body/' + request.public_body.url_name, + '/request/' + request.url_title, + '/details/request/' + request.url_title, + '^/list', + '/feed/request/' + request.url_title, + '^/feed/list/', + '/feed/body/' + request.public_body.url_name, + '/feed/user/' + request.user.url_name, + '/user/' + request.user.url_name, + '/user/' + request.user.url_name + '/wall' + ]) + end + end + describe '#foi_attachments' do subject { info_request.foi_attachments } diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index c6bbf962d2e..96d7274a05b 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -197,6 +197,14 @@ end + describe '.cached_urls' do + it 'includes the correct paths' do + body = FactoryBot.create(:public_body) + body_path = "/body/" + body.url_name + expect(body.cached_urls).to eq([body_path, '/body', '^/body/list']) + end + end + describe '#save' do subject { public_body.save } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ec086c83f01..ea26e1b7f63 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -741,6 +741,13 @@ def create_user(options = {}) end end + describe '.cached_urls' do + it 'includes the correct paths' do + user = FactoryBot.create(:user) + expect(user.cached_urls).to eq(["/user/" + user.url_name]) + end + end + describe '#transactions' do it 'returns a TransactionCalculator with the default transaction set' do