Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load government data in an asynchronous task #1522

Merged
merged 30 commits into from
Dec 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
62bab09
Create a module for BulkData
kevindew Dec 9, 2019
a764576
BulkData cache singleton class
kevindew Dec 9, 2019
659c391
Adapt Government model for loading from Publishing API
kevindew Dec 9, 2019
ec71269
Provide a to_h method on InitializeWithHash
kevindew Dec 9, 2019
77a50c2
GovernmentRepository class for BulkData
kevindew Dec 9, 2019
2e55195
Set up spec helpers for bulk data
kevindew Dec 9, 2019
10bd447
Transition edition to use BulkData::GovernmentRepository
kevindew Dec 9, 2019
5a5fc1a
Use new government mocking approach for PreviewService::Payload
kevindew Dec 9, 2019
5aff300
Load governments from repository for PublishService
kevindew Dec 9, 2019
3bac691
Minor refactoring of document request spec
kevindew Dec 9, 2019
49240a9
Fix incorrect I18n call
kevindew Dec 9, 2019
dbe6b81
Use government repository in edit edition service
kevindew Dec 9, 2019
59e57e4
Switch government name to title
kevindew Dec 9, 2019
7ede62a
Stub governments for tests
kevindew Dec 9, 2019
5cc6f84
Re-populate governments when working with future dates
kevindew Dec 9, 2019
06d58ae
Adjust request/feature tests for change in government trait
kevindew Dec 12, 2019
edcf545
Helper method to run ActiveJobs exclusively
kevindew Dec 10, 2019
c59ee81
Default to fake sidekiq testing
kevindew Dec 10, 2019
070b673
Stub government data for edition filter specs
kevindew Dec 10, 2019
a7fd0d4
Add document live trait that sets first_published_at
kevindew Dec 10, 2019
4f42b1b
Use government repository for ResyncService
kevindew Dec 10, 2019
3d3245c
Minor refactoring of ResyncService specs
kevindew Dec 10, 2019
69f4964
Add job to populate bulk data asynchronously
kevindew Dec 10, 2019
dfb1eb2
Asynchronously load bulk data if it is not available
kevindew Dec 11, 2019
b177dc2
Add an error view for BulkData::LocalDataUnavailableError
kevindew Dec 11, 2019
d2ad3e7
Remove hardcoded government config
kevindew Dec 11, 2019
be7cb1f
Use specific government in history mode spec
kevindew Dec 12, 2019
8399dd5
Use local cache Rack middleware with BulkData::Cache
kevindew Dec 12, 2019
fa27e4c
Configure Redis for resiliency and reporting
kevindew Dec 16, 2019
d395aa5
Don't report BulkData::RemoteDataUnavailableError on server errors
kevindew Dec 16, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ class ApplicationController < ActionController::Base
end
end

rescue_from BulkData::LocalDataUnavailableError do |error|
GovukError.notify(error)

render "errors/local_data_unavailable",
locals: { error: error },
status: :service_unavailable
end

def rendering_context
request.headers["Content-Publisher-Rendering-Context"] || "application"
end
Expand Down
18 changes: 18 additions & 0 deletions app/jobs/application_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,22 @@ class ApplicationJob < ActiveJob::Base
def self.discard_and_log(exception)
discard_on(exception) { |_, error| Rails.logger.warn(error) }
end

def run_exclusively(transaction: true, lock_name: self.class.name)
name = "content-publisher:#{lock_name}"
leenagupte marked this conversation as resolved.
Show resolved Hide resolved
options = { timeout_seconds: 0, transaction: transaction }
result = if transaction
ApplicationRecord.transaction do
ApplicationRecord.with_advisory_lock_result(name, options) { yield }
end
else
ApplicationRecord.with_advisory_lock_result(name, options) { yield }
end

unless result.lock_was_acquired?
logger.info("Job skipped as exclusive lock '#{name}' could not be acuqired")
end

result.result
end
end
2 changes: 1 addition & 1 deletion app/jobs/asset_cleanup_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

class AssetCleanupJob < ApplicationJob
def perform
ApplicationRecord.with_advisory_lock("content-publisher_asset-cleanup", timeout_seconds: 0) do
run_exclusively do
clean_image_assets
clean_file_attachment_assets
end
Expand Down
13 changes: 13 additions & 0 deletions app/jobs/populate_bulk_data_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

class PopulateBulkDataJob < ApplicationJob
retry_on BulkData::RemoteDataUnavailableError do |_job, error|
GovukError.notify(error) unless error.cause.is_a?(GdsApi::HTTPServerError)
end

def perform
run_exclusively do
BulkData::GovernmentRepository.new.populate_cache(older_than: 5.minutes.ago)
end
end
end
9 changes: 7 additions & 2 deletions app/models/concerns/initialize_with_hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@
module InitializeWithHash
include ActiveSupport::Concern

def initialize(params = {})
params.each do |key, value|
attr_reader :attributes

def initialize(attributes = {})
@attributes = attributes
@attributes.each do |key, value|
instance_variable_set("@#{key}", value)
end
end

alias_method :to_h, :attributes
end
8 changes: 5 additions & 3 deletions app/models/edition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,12 @@ class Edition < ApplicationRecord
end

scope :history_mode, ->(history_mode = true) do
repository = BulkData::GovernmentRepository.new
if history_mode
political.where.not(government_id: [nil, Government.current.content_id])
political.where.not(government_id: [nil, repository.current.content_id])
else
political(false)
.or(political.where(government_id: [nil, Government.current.content_id]))
.or(political.where(government_id: [nil, repository.current.content_id]))
end
end

Expand All @@ -133,7 +134,8 @@ def history_mode?
def government
return unless government_id

Government.find(government_id)
@government_repository ||= BulkData::GovernmentRepository.new
@government_repository.find(government_id)
ChrisBAshton marked this conversation as resolved.
Show resolved Hide resolved
end

def public_first_published_at
Expand Down
42 changes: 14 additions & 28 deletions app/models/government.rb
Original file line number Diff line number Diff line change
@@ -1,49 +1,35 @@
# frozen_string_literal: true

class Government
include ActiveModel::Model
include InitializeWithHash

def self.find(content_id)
government = all.find { |g| g.content_id == content_id }
government || (raise "Government #{content_id} not found")
end

def self.for_date(date)
all.find { |government| government.covers?(date) } if date
end

def self.current
for_date(Date.current)
end

def self.past
all.reject(&:current?)
end

def self.all
@all ||= YAML.load_file(Rails.root.join("config/governments.yml"))
.map { |hash| new(hash) }
end

attr_accessor :content_id, :slug, :name, :start_date, :end_date
attr_accessor :content_id, :locale, :title, :details

def ==(other)
content_id == other.content_id
content_id == other.content_id && locale == other.locale
end

alias_method :eql?, :==

def covers?(date)
return false if date < start_date
return false if date < started_on
# Most end dates in Whitehall are the last date of a government so we
# treat the date as going up to 23:59:59 on the day by appending 1 day to
# the date
return false if end_date && date >= (end_date + 1)
return false if ended_on && date >= (ended_on + 1)

true
end

def started_on
@started_on ||= Date.parse(details["started_on"])
end

def ended_on
@ended_on ||= Date.parse(details["ended_on"]) if details["ended_on"]
end

def current?
self == self.class.current
details["current"] == true
end
end
4 changes: 3 additions & 1 deletion app/services/edit_edition_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ def determine_political
end

def associate_with_government
edition.government_id = Government.for_date(edition.public_first_published_at)&.content_id
repository = BulkData::GovernmentRepository.new
date = edition.public_first_published_at
edition.government_id = repository.for_date(date)&.content_id
end
end
5 changes: 3 additions & 2 deletions app/services/publish_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ def publish_assets(live_edition)
def associate_with_government
return if edition.government

repository = BulkData::GovernmentRepository.new
government = if edition.public_first_published_at
Government.for_date(edition.public_first_published_at)
repository.for_date(edition.public_first_published_at)
else
Government.current
repository.current
end
edition.assign_attributes(government_id: government&.content_id)

Expand Down
5 changes: 4 additions & 1 deletion app/services/resync_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ def sync_draft_edition
end

def set_political_and_government(edition)
repository = BulkData::GovernmentRepository.new
government = repository.for_date(edition.public_first_published_at)

edition.update!(
revision_synced: false,
system_political: PoliticalEditionIdentifier.new(edition).political?,
government_id: Government.for_date(edition.public_first_published_at)&.content_id,
government_id: government&.content_id,
)
end

Expand Down
12 changes: 4 additions & 8 deletions app/views/documents/show/_historical_notice.html.erb
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
<%= render "govuk_publishing_components/components/notice",
{
title: I18n.t(
"documents.show.historical.title",
document_type: @edition.document_type.label.downcase
),
description_text: I18n.t(
"documents.show.historical.description",
government_name: @edition.government.name
),
title: t("documents.show.historical.title",
document_type: @edition.document_type.label.downcase),
description_text: t("documents.show.historical.description",
government_name: @edition.government.title),
}
%>
22 changes: 22 additions & 0 deletions app/views/errors/local_data_unavailable.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<% if Rails.env.development? %>
<%
title = "Expected cached data missing"
backtrace = error.backtrace.map { |line| "- <code>#{line}</code>" }.join("\n")
body_govspeak = <<~GOVSPEAK
Content Publisher relies on background jobs to load data from other GOV.UK
applications. To resolve this error ensure that the worker process is
running for Content Publisher.

Alternatively or if you are experiencing issues with the worker, you can
run the job directly in a rails console which should tell you more:
`PopulateBulkDataJob.perform_now`.

## Backtrace

#{backtrace}
GOVSPEAK
%>
<%= render partial: "errors/error", locals: { title: title, body_govspeak: body_govspeak } %>
<% else %>
<%= render partial: "errors/error", locals: t("errors.local_data_unavailable") %>
<% end %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole template is smashing 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 😁

Loading