From 62bab09104a2aafdf8ccbe83f73ab4de0ca35ecb Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 17:00:28 +0000
Subject: [PATCH 01/30] Create a module for BulkData

Bulk data is the namespace that we will be using for download large
amounts of data periodically for use in Content Publisher. Some examples
of candidates for this are topics, contacts and organisations. These are
data entities where we need to download all of the Publishing API's data
rather than a small sampling of on demand data.

Previously we have loaded this data on demand and it risks periodic
timeouts for the application. As this data can be loaded in numerous
places it also makes it difficult to determine performance issues in the
application as it may be a contributing factor to any other tasks we
think may be slow.

This module contains two common exceptions that we're anticipating to
use. LocalDataUnavailableError is for situations when we're expecting
data to be available in our cache but it is not present.
RemoteDataUnavailableError is for when we try to load remote data but
this fails.
---
 lib/bulk_data.rb | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 lib/bulk_data.rb

diff --git a/lib/bulk_data.rb b/lib/bulk_data.rb
new file mode 100644
index 0000000000..21cc4b36fa
--- /dev/null
+++ b/lib/bulk_data.rb
@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+module BulkData
+  class LocalDataUnavailableError < RuntimeError; end
+  class RemoteDataUnavailableError < RuntimeError; end
+end

From a764576e87d6b0ea0e3f651cfaa1797dd03b0346 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 17:00:42 +0000
Subject: [PATCH 02/30] BulkData cache singleton class

This class is intended to be the entry point for writing any BulkData.
It uses the Rails Redis cache as an underlying store.

Using Redis was a design choice for the following reasons:

- Redis offers caching across multiple GOV.UK boxes, so one write will
cover all machines
- Redis is fast to read and write from

And using the Rails cache implementation offered the following
advantages:

- Can be configured with [middleware][1] to avoid need to hit Redis multiple
  times in a request
- Redis namespacing
- Automatic serialisation/deserialisation

An important concern is that we need to ignore the guidance Rails
suggests to use a LRU redis cache, as we don't want to have keys
automatically cleared out of redis. Instead we just want them to expire
naturally.

The methods on this class allow adding an item to the cache, removing an
item, removing all items.

On top of this there are time based methods which are intended to be
used to determine when an item was added to cache. This is to enable
checking how old something is when deciding whether to add something or
not. This is a feature Redis doesn't have by default so it's quite
simple to just add this a different entry, the Rails cache entry
actually has some info about created time, but I have no idea how to
access it and also think it's probably nicer to not have to pull out the
whole cache entry to check it.

[1]: https://github.com/rails/rails/blob/9926fd86626690746c7ee78174c4d3577bdfa58d/activesupport/lib/active_support/cache/redis_cache_store.rb#L44
---
 lib/bulk_data/cache.rb           | 52 ++++++++++++++++++
 spec/lib/bulk_data/cache_spec.rb | 90 ++++++++++++++++++++++++++++++++
 spec/spec_helper.rb              |  1 +
 3 files changed, 143 insertions(+)
 create mode 100644 lib/bulk_data/cache.rb
 create mode 100644 spec/lib/bulk_data/cache_spec.rb

diff --git a/lib/bulk_data/cache.rb b/lib/bulk_data/cache.rb
new file mode 100644
index 0000000000..e39ba67e5f
--- /dev/null
+++ b/lib/bulk_data/cache.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+module BulkData
+  class Cache
+    include Singleton
+
+    class NoEntryError < RuntimeError; end
+
+    class << self
+      delegate :cache, to: :instance
+    end
+
+    attr_reader :cache
+
+    def self.write(key, value)
+      cache.write(key, value, expires_in: 24.hours)
+      cache.write("#{key}:created", Time.current, expires_in: 24.hours)
+    end
+
+    def self.read(key)
+      cache.read(key) || (raise NoEntryError)
+    end
+
+    def self.written_at(key)
+      cache.read("#{key}:created")
+    end
+
+    def self.written_after?(key, time)
+      created = written_at(key)
+      return false unless created
+
+      created > time
+    end
+
+    def self.delete(key)
+      cache.delete(key)
+      cache.delete("#{key}:created")
+    end
+
+    def self.clear
+      cache.clear
+    end
+
+  private
+
+    def initialize
+      @cache = ActiveSupport::Cache::RedisCacheStore.new(
+        namespace: "content-publisher:bulk-data-cache-#{Rails.env}",
+      )
+    end
+  end
+end
diff --git a/spec/lib/bulk_data/cache_spec.rb b/spec/lib/bulk_data/cache_spec.rb
new file mode 100644
index 0000000000..e3b9a1f219
--- /dev/null
+++ b/spec/lib/bulk_data/cache_spec.rb
@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+RSpec.describe BulkData::Cache do
+  describe ".write" do
+    it "adds an entry that expires in 24 hours" do
+      BulkData::Cache.write("key", "value")
+      expect(BulkData::Cache.cache.read("key")).to eq("value")
+
+      travel_to(25.hours.from_now) do
+        expect(BulkData::Cache.cache.read("key")).to be_nil
+      end
+    end
+
+    it "sets the current time as the age, which also expires" do
+      freeze_time do
+        BulkData::Cache.write("key", "value")
+        expect(BulkData::Cache.cache.read("key:created")).to eq(Time.current)
+      end
+
+      travel_to(25.hours.from_now) do
+        expect(BulkData::Cache.cache.read("key:created")).to be_nil
+      end
+    end
+  end
+
+  describe ".read" do
+    it "returns a value when one exists" do
+      BulkData::Cache.write("key", "value")
+      expect(BulkData::Cache.read("key")).to eq("value")
+    end
+
+    it "raises a NoEntryError when there isn't an entry" do
+      expect { BulkData::Cache.read("key") }
+        .to raise_error(BulkData::Cache::NoEntryError)
+    end
+  end
+
+  describe ".written_at" do
+    it "returns the time the entry was created" do
+      time = 2.hours.ago.change(usec: 0)
+      travel_to(time) { BulkData::Cache.write("key", "value") }
+      expect(BulkData::Cache.written_at("key")).to eq(time)
+    end
+
+    it "returns nil when the entry doesn't exist" do
+      expect(BulkData::Cache.written_at("key")).to be_nil
+    end
+  end
+
+  describe ".written_after?" do
+    it "returns true if the cache was written after the time" do
+      travel_to(3.minutes.ago) { BulkData::Cache.write("key", "value") }
+      expect(BulkData::Cache.written_after?("key", 5.minutes.ago)).to be true
+    end
+
+    it "returns false if the cache was written before the time" do
+      travel_to(10.minutes.ago) { BulkData::Cache.write("key", "value") }
+      expect(BulkData::Cache.written_after?("key", 5.minutes.ago)).to be false
+    end
+
+    it "returns false if the cache entry doesn't exist" do
+      expect(BulkData::Cache.written_after?("key", 5.minutes.ago)).to be false
+    end
+  end
+
+  describe ".delete" do
+    it "deletes an entry" do
+      BulkData::Cache.write("key", "value")
+      expect { BulkData::Cache.delete("key") }
+        .to change { BulkData::Cache.cache.read("key") }.to(nil)
+    end
+
+    it "deletes the created value of an entry" do
+      BulkData::Cache.write("key", "value")
+      expect { BulkData::Cache.delete("key") }
+        .to change { BulkData::Cache.cache.read("key:created") }.to(nil)
+    end
+  end
+
+  describe ".clear" do
+    it "deletes all entries" do
+      BulkData::Cache.write("key", "value")
+      BulkData::Cache.write("another-key", "different value")
+
+      expect { BulkData::Cache.clear }
+        .to change { BulkData::Cache.cache.read("key") }.to(nil)
+        .and change { BulkData::Cache.cache.read("another-key") }.to(nil)
+    end
+  end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 921c377738..5e485a62cd 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -51,6 +51,7 @@
   config.before :each do
     Sidekiq::Worker.clear_all
     ActionMailer::Base.deliveries.clear
+    BulkData::Cache.clear
   end
 
   config.after :each, type: ->(spec) { spec.in?(%i[feature request]) } do

From 659c391fd2c56c2240459c1fbef4887ad0e3ed4c Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 17:44:30 +0000
Subject: [PATCH 03/30] Adapt Government model for loading from Publishing API

This sets up this model to work with data from the Publishing API. It no
longer has class methods to look up the model as these are going to be
on a different class.

The input we expect from the Publishing API is a hash of base_path,
content_id, details, locale and title. We can look up started_on,
ended_on and current dates from the details hash.
---
 app/models/government.rb             |  42 +++------
 spec/factories/government_factory.rb |  28 +++++-
 spec/models/government_spec.rb       | 136 +++++++++------------------
 3 files changed, 80 insertions(+), 126 deletions(-)

diff --git a/app/models/government.rb b/app/models/government.rb
index b248f7c2f3..4c202a4c35 100644
--- a/app/models/government.rb
+++ b/app/models/government.rb
@@ -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
diff --git a/spec/factories/government_factory.rb b/spec/factories/government_factory.rb
index 9c44cd62f6..e78af133cb 100644
--- a/spec/factories/government_factory.rb
+++ b/spec/factories/government_factory.rb
@@ -4,11 +4,29 @@
   factory :government, class: Government do
     skip_create
 
+    transient do
+      started_on { Date.parse("2015-05-08") }
+      ended_on { nil }
+      current { ended_on.nil? }
+    end
+
     content_id { SecureRandom.uuid }
-    name { SecureRandom.alphanumeric(8) }
-    slug { name.parameterize }
-    start_date { Date.parse("2015-05-08") }
-    end_date { nil }
-    initialize_with { new(attributes) }
+    locale { "en" }
+    title { SecureRandom.alphanumeric(8) }
+
+    details do
+      {
+        "started_on" => started_on.rfc3339,
+        "ended_on" => ended_on&.rfc3339,
+        "current" => current,
+      }
+    end
+
+    trait :past do
+      started_on { Date.parse("2010-05-12") }
+      ended_on { Date.parse("2015-05-08") }
+    end
+
+    initialize_with { new(attributes.stringify_keys) }
   end
 end
diff --git a/spec/models/government_spec.rb b/spec/models/government_spec.rb
index 88f6fcc767..a5409823ba 100644
--- a/spec/models/government_spec.rb
+++ b/spec/models/government_spec.rb
@@ -1,104 +1,39 @@
 # frozen_string_literal: true
 
 RSpec.describe Government do
-  describe ".all" do
-    it "returns a collection of governments" do
-      expect(Government.all).not_to be_empty
-      expect(Government.all).to all be_a(Government)
-    end
-  end
-
-  describe ".find" do
-    let(:content_id) { SecureRandom.uuid }
-    let(:government) { build(:government, content_id: content_id) }
-    before { allow(Government).to receive(:all).and_return([government]) }
-
-    it "finds a government by content id" do
-      expect(Government.find(content_id)).to be government
-    end
-
-    it "raises an error if there isn't a government for the id" do
-      missing_content_id = SecureRandom.uuid
-      expect { Government.find(missing_content_id) }
-        .to raise_error(RuntimeError, "Government #{missing_content_id} not found")
-    end
-  end
-
-  describe ".for_date" do
-    let(:current_government) do
-      build(:government,
-            start_date: Date.parse("2018-12-01"),
-            end_date: nil)
-    end
+  describe "#==" do
+    it "returns true when content_id and locale are equal" do
+      government = build(:government, started_on: Time.zone.today)
+      other = build(:government,
+                    content_id: government.content_id,
+                    locale: government.locale,
+                    started_on: Time.zone.yesterday)
 
-    let(:past_government) do
-      build(:government,
-            start_date: Date.parse("2018-01-31"),
-            end_date: Date.parse("2018-12-01"))
+      expect(government).to eq(other)
     end
 
-    before do
-      allow(Government).to receive(:all)
-                       .and_return([current_government, past_government])
+    it "returns false when content_id doesn't match" do
+      government = build(:government, content_id: SecureRandom.uuid, locale: "en")
+      other = build(:government, content_id: SecureRandom.uuid, locale: "en")
+      expect(government).not_to eq(other)
     end
 
-    it "returns a government when there is one for the date" do
-      expect(Government.for_date(Date.parse("2019-06-01"))).to eq current_government
-    end
-
-    it "returns nil when there isn't a government for the date" do
-      expect(Government.for_date(Date.parse("2017-06-01"))).to be_nil
-    end
-
-    it "opts for the more recent government when there is a date conflict" do
-      expect(Government.for_date(Date.parse("2018-12-01"))).to eq current_government
-    end
-
-    it "accepts a nil date" do
-      expect(Government.for_date(nil)).to be_nil
-    end
-  end
-
-  describe ".current" do
-    let(:current_government) do
-      build(:government,
-            start_date: Date.parse("2018-12-01"),
-            end_date: nil)
-    end
-
-    let(:past_government) do
-      build(:government,
-            start_date: Date.parse("2018-01-31"),
-            end_date: Date.parse("2018-12-01"))
-    end
-
-    before do
-      allow(Government).to receive(:all)
-                       .and_return([current_government, past_government])
-    end
-
-    it "returns the current government" do
-      expect(Government.current).to eq current_government
-    end
-  end
-
-  describe ".past" do
-    it "returns the past governments" do
-      past = Government.past
-      expect(past).not_to be_empty
-      expect(past).to all be_a(Government)
-      expect(past).not_to include(Government.current)
+    it "returns false when locale doesn't match" do
+      content_id = SecureRandom.uuid
+      english = build(:government, content_id: content_id, locale: "en")
+      french = build(:government, content_id: content_id, locale: "fr")
+      expect(english).not_to eq(french)
     end
   end
 
   describe "#covers?" do
     let(:government) do
-      build(:government, start_date: start_date, end_date: end_date)
+      build(:government, started_on: started_on, ended_on: ended_on)
     end
 
     context "when there isn't an end date" do
-      let(:start_date) { Date.parse("2019-11-18") }
-      let(:end_date) { nil }
+      let(:started_on) { Date.parse("2019-11-18") }
+      let(:ended_on) { nil }
 
       it "returns false before the start date" do
         expect(government.covers?(Date.parse("2019-11-01"))).to be false
@@ -114,8 +49,8 @@
     end
 
     context "when there is a start date and an end date" do
-      let(:start_date) { Date.parse("2019-11-18") }
-      let(:end_date) { Date.parse("2019-11-20") }
+      let(:started_on) { Date.parse("2019-11-18") }
+      let(:ended_on) { Date.parse("2019-11-20") }
 
       it "returns false before the start date" do
         expect(government.covers?(Date.parse("2019-11-17"))).to be false
@@ -140,16 +75,31 @@
     end
   end
 
-  describe "#current?" do
-    let(:government) { build(:government) }
+  describe "#started_on" do
+    it "returns a date" do
+      expect(build(:government).started_on).to be_a(Date)
+    end
+  end
+
+  describe "#ended_on" do
+    it "returns a date when ended on is set" do
+      government = build(:government, ended_on: Time.zone.yesterday)
+      expect(government.ended_on).to be_a(Date)
+    end
 
-    it "returns true when the government is the current one" do
-      allow(Government).to receive(:current).and_return(government)
-      expect(government.current?).to be true
+    it "returns nil when an end date isn't set" do
+      government = build(:government, ended_on: nil)
+      expect(government.ended_on).to be_nil
+    end
+  end
+
+  describe "#current?" do
+    it "returns true when the government is marked as the current one" do
+      expect(build(:government, current: true).current?).to be true
     end
 
     it "returns false when the government is not the current one" do
-      expect(government.current?).to be false
+      expect(build(:government, current: false).current?).to be false
     end
   end
 end

From ec71269696c02af98e5fe054b2f7c600272fdb5b Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 19:41:49 +0000
Subject: [PATCH 04/30] Provide a to_h method on InitializeWithHash

This allows classes that mix this module in to be serialised to a hash.
This has been added so that instances that use this module can be
serialised to fake cache data.
---
 app/models/concerns/initialize_with_hash.rb | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/app/models/concerns/initialize_with_hash.rb b/app/models/concerns/initialize_with_hash.rb
index 44c6cba5e1..f143eaf6fd 100644
--- a/app/models/concerns/initialize_with_hash.rb
+++ b/app/models/concerns/initialize_with_hash.rb
@@ -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

From 77a50c2933330b218a3cea197e883db75344d60c Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 17:46:51 +0000
Subject: [PATCH 05/30] GovernmentRepository class for BulkData

The purpose of this class is to mediate communication between Publishing
API and our cache adapter for governments.

It writes entries to the cache using a cache key that is defined as a
constant in this class. This is named as "government-v1" to indicate
that this is version 1 of the data we are caching. If we are to make any
backwards incompatible changes to the data we expect (such as adding an
extra mandatory field) then we will need to increment this number. This
will ensure that when the code change is deployed the old cache will not
be used anymore and will expire naturally.

It provides a number of methods to finding a particular government and
a method to populate the cache.
---
 lib/bulk_data/government_repository.rb        |  49 ++++++
 .../bulk_data/government_repository_spec.rb   | 157 ++++++++++++++++++
 2 files changed, 206 insertions(+)
 create mode 100644 lib/bulk_data/government_repository.rb
 create mode 100644 spec/lib/bulk_data/government_repository_spec.rb

diff --git a/lib/bulk_data/government_repository.rb b/lib/bulk_data/government_repository.rb
new file mode 100644
index 0000000000..a6b5761f99
--- /dev/null
+++ b/lib/bulk_data/government_repository.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module BulkData
+  class GovernmentRepository
+    CACHE_KEY = "government-v1"
+
+    def find(content_id)
+      government = all.find { |g| g.content_id == content_id }
+      government || (raise "Government #{content_id} not found")
+    end
+
+    def for_date(date)
+      all.find { |g| g.covers?(date) } if date
+    end
+
+    def current
+      all.find(&:current?)
+    end
+
+    def past
+      all.reject(&:current?)
+    end
+
+    def all
+      @all ||= Cache.read(CACHE_KEY)
+                    .map { |data| Government.new(data) }
+                    .sort_by(&:started_on)
+    rescue Cache::NoEntryError
+      raise LocalDataUnavailableError
+    end
+
+    def populate_cache(older_than: nil)
+      return if older_than && Cache.written_after?(CACHE_KEY, older_than)
+
+      data = GdsApi.publishing_api_v2
+                   .get_paged_editions(document_types: %w[government],
+                                       fields: %w[content_id locale title details],
+                                       states: %w[published],
+                                       locale: "en",
+                                       per_page: 1000)
+                   .inject([]) { |memo, page| memo + page["results"] }
+
+      @all = nil
+      Cache.write(CACHE_KEY, data)
+    rescue GdsApi::BaseError
+      raise RemoteDataUnavailableError
+    end
+  end
+end
diff --git a/spec/lib/bulk_data/government_repository_spec.rb b/spec/lib/bulk_data/government_repository_spec.rb
new file mode 100644
index 0000000000..8372795437
--- /dev/null
+++ b/spec/lib/bulk_data/government_repository_spec.rb
@@ -0,0 +1,157 @@
+# frozen_string_literal: true
+
+RSpec.describe BulkData::GovernmentRepository do
+  let(:repository) { BulkData::GovernmentRepository.new }
+  let(:cache_key) { BulkData::GovernmentRepository::CACHE_KEY }
+
+  describe "#find" do
+    let(:government) { build(:government) }
+    before { BulkData::Cache.write(cache_key, [government.to_h]) }
+
+    it "can find a government for a particular content id" do
+      expect(repository.find(government.content_id)).to eq(government)
+    end
+
+    it "raises an error when the government isn't known" do
+      content_id = SecureRandom.uuid
+      expect { repository.find(content_id) }
+        .to raise_error("Government #{content_id} not found")
+    end
+  end
+
+  describe "#for_date" do
+    let(:government) do
+      build(:government, started_on: 3.weeks.ago, ended_on: 1.week.ago)
+    end
+
+    before { BulkData::Cache.write(cache_key, [government.to_h]) }
+
+    it "returns the government for that date" do
+      expect(repository.for_date(2.weeks.ago)).to eq(government)
+    end
+
+    it "returns nil if there isn't a government covering the date" do
+      expect(repository.for_date(4.weeks.ago)).to be_nil
+    end
+
+    it "returns nil if there isn't a date" do
+      expect(repository.for_date(nil)).to be_nil
+    end
+  end
+
+  describe "#current" do
+    it "returns the current government when there is one" do
+      government = build(:government, ended_on: nil)
+      BulkData::Cache.write(cache_key, [government.to_h])
+      expect(repository.current).to eq(government)
+    end
+
+    it "returns nil when there isn't a current government" do
+      government = build(:government, :past)
+      BulkData::Cache.write(cache_key, [government.to_h])
+      expect(repository.current).to be_nil
+    end
+  end
+
+  describe "#past" do
+    it "returns all the governments that aren't current" do
+      government_a = build(:government,
+                           started_on: Date.parse("2018-11-11"),
+                           ended_on: nil)
+      government_b = build(:government,
+                           started_on: Date.parse("2015-11-11"),
+                           ended_on: Date.parse("2018-11-11"))
+      government_c = build(:government,
+                           started_on: Date.parse("2012-11-11"),
+                           ended_on: Date.parse("2015-11-11"))
+
+      BulkData::Cache.write(
+        cache_key,
+        [government_a.to_h, government_b.to_h, government_c.to_h],
+      )
+
+      expect(repository.past).to eq([government_c, government_b])
+    end
+
+    it "returns empty if there are no past govenments" do
+      government = build(:government, ended_on: nil)
+      BulkData::Cache.write(cache_key, [government.to_h])
+      expect(repository.past).to be_empty
+    end
+  end
+
+  describe "#all" do
+    it "returns an array of governments sorted by started on date" do
+      government_a = build(:government,
+                           started_on: Date.parse("2018-11-11"),
+                           ended_on: Date.parse("2019-11-11"))
+      government_b = build(:government,
+                           started_on: Date.parse("2012-11-11"),
+                           ended_on: Date.parse("2015-11-11"))
+      government_c = build(:government,
+                           started_on: Date.parse("2015-11-11"),
+                           ended_on: Date.parse("2018-11-11"))
+      BulkData::Cache.write(
+        cache_key,
+        [government_a.to_h, government_b.to_h, government_c.to_h],
+      )
+
+      expect(repository.all).to eq([government_b, government_c, government_a])
+    end
+
+    it "raises a LocalDataUnavailableError when there isn't data" do
+      expect { repository.all }
+        .to raise_error(BulkData::LocalDataUnavailableError)
+    end
+  end
+
+  describe "#populate_cache" do
+    let(:governments) do
+      [build(:government).to_h, build(:government, :past).to_h]
+    end
+
+    let!(:get_editions_request) do
+      stub_publishing_api_get_editions(
+        governments,
+        document_types: %w[government],
+        fields: %w[content_id locale title details],
+        states: %w[published],
+        locale: "en",
+        per_page: 1000,
+      )
+    end
+
+    it "populates the cache with governments from the Publishing API" do
+      expect(BulkData::Cache).to receive(:write).with(cache_key, governments)
+      repository.populate_cache
+      expect(get_editions_request).to have_been_requested
+    end
+
+    it "raises a RemoteDataUnavailableError when Publishing API is unavailable" do
+      stub_publishing_api_isnt_available
+
+      expect { repository.populate_cache }
+        .to raise_error(BulkData::RemoteDataUnavailableError)
+    end
+
+    it "resets the all data stored on the class" do
+      BulkData::Cache.write(cache_key, [])
+      expect { repository.populate_cache }
+        .to change { repository.all.count }.from(0).to(2)
+    end
+
+    it "re-populates the cache when it was updated before the older_than time" do
+      travel_to(10.minutes.ago) { repository.populate_cache }
+
+      expect(BulkData::Cache).to receive(:write)
+      repository.populate_cache(older_than: 5.minutes.ago)
+    end
+
+    it "does nothing when cache was updated after the older_than time" do
+      travel_to(2.minutes.ago) { repository.populate_cache }
+
+      expect(BulkData::Cache).not_to receive(:write)
+      repository.populate_cache(older_than: 5.minutes.ago)
+    end
+  end
+end

From 2e551950d24728ec309b29f3137c5fb4f39f1f70 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 19:53:42 +0000
Subject: [PATCH 06/30] Set up spec helpers for bulk data

This clears the BulkData::Cache after every spec and helpers to populate
the cache before tests.

Default governments are set up for the most common case where we just
need to know about the current and previous governments.

It also provides helper methods for current and past government as these
are likely to be needed quite frequently.
---
 spec/spec_helper.rb              |  1 +
 spec/support/bulk_data_helper.rb | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 spec/support/bulk_data_helper.rb

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 5e485a62cd..1cc315a879 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -36,6 +36,7 @@
   config.include GdsApi::TestHelpers::AssetManager
   config.include GovukSchemas::RSpecMatchers
   config.include AuthenticationHelper, type: ->(spec) { spec.in?(%i[feature request]) }
+  config.include BulkDataHelper
 
   config.fixture_path = "#{::Rails.root}/spec/fixtures"
 
diff --git a/spec/support/bulk_data_helper.rb b/spec/support/bulk_data_helper.rb
new file mode 100644
index 0000000000..0fb98b49c8
--- /dev/null
+++ b/spec/support/bulk_data_helper.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module BulkDataHelper
+  def populate_default_government_bulk_data
+    current_government = build(:government)
+    past_government = build(:government, :past)
+
+    populate_government_bulk_data(current_government, past_government)
+  end
+
+  def populate_government_bulk_data(*governments)
+    BulkData::Cache.write(BulkData::GovernmentRepository::CACHE_KEY,
+                          governments.map(&:to_h))
+  end
+
+  def current_government
+    BulkData::GovernmentRepository.new.current
+  end
+
+  def past_government
+    BulkData::GovernmentRepository.new.past&.first
+  end
+end

From 10bd447ba3ed21249b3a00e2406f149061404131 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 20:06:35 +0000
Subject: [PATCH 07/30] Transition edition to use
 BulkData::GovernmentRepository

This creates instances of the GovernmentRepository class whenever we
need to load governments in the context of an edition. The factory has
been updated to reflect that governments can't be determined without an
external dependency and thus governments are always injected.

This does raise the prospect of some potential performance concerns for
the amount of classes initialised if we are frequently looking up
government information. Mostly though I expect these things to not occur
multiple times in the same request so I don't think it'll cause any
actual performance issues.
---
 app/models/edition.rb             |  8 +++++---
 spec/factories/edition_factory.rb | 10 ++--------
 spec/models/edition_spec.rb       | 22 +++++++++++++---------
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/app/models/edition.rb b/app/models/edition.rb
index 72e01be534..0742a8924b 100644
--- a/app/models/edition.rb
+++ b/app/models/edition.rb
@@ -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
 
@@ -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)
   end
 
   def public_first_published_at
diff --git a/spec/factories/edition_factory.rb b/spec/factories/edition_factory.rb
index a4b0dd2d38..6be199f957 100644
--- a/spec/factories/edition_factory.rb
+++ b/spec/factories/edition_factory.rb
@@ -5,6 +5,7 @@
     last_edited_at { Time.current }
     current { true }
     live { false }
+    government_id { government&.content_id }
     revision_synced { true }
     association :created_by, factory: :user
 
@@ -19,6 +20,7 @@
       image_revisions { [] }
       file_attachment_revisions { [] }
       first_published_at { nil }
+      government { nil }
     end
 
     after(:build) do |edition, evaluator|
@@ -203,13 +205,5 @@
     trait :not_political do
       system_political { false }
     end
-
-    trait :current_government do
-      government_id { Government.current.content_id }
-    end
-
-    trait :past_government do
-      government_id { Government.past.first.content_id }
-    end
   end
 end
diff --git a/spec/models/edition_spec.rb b/spec/models/edition_spec.rb
index 61b8bb1c77..76d26bb235 100644
--- a/spec/models/edition_spec.rb
+++ b/spec/models/edition_spec.rb
@@ -41,9 +41,11 @@
   end
 
   describe ".history_mode" do
-    let!(:political_past_government) { create(:edition, :political, :past_government) }
-    let!(:political_current_government) { create(:edition, :political, :current_government) }
-    let!(:political_no_government) { create(:edition, :political, government_id: nil) }
+    before { populate_default_government_bulk_data }
+
+    let!(:political_past_government) { create(:edition, :political, government: past_government) }
+    let!(:political_current_government) { create(:edition, :political, government: current_government) }
+    let!(:political_no_government) { create(:edition, :political, government: nil) }
     let!(:not_political) { create(:edition, :not_political) }
 
     it "defaults to scoping to only history mode editions" do
@@ -82,8 +84,10 @@
   end
 
   describe "#history_mode?" do
+    before { populate_default_government_bulk_data }
+
     it "returns true when political and associated with a previous government" do
-      edition = build(:edition, :political, :past_government)
+      edition = build(:edition, :political, government: past_government)
       expect(edition.history_mode?).to be(true)
     end
 
@@ -93,26 +97,26 @@
     end
 
     it "returns false when the edition isn't associated with a government" do
-      edition = build(:edition, :political, government_id: nil)
+      edition = build(:edition, :political, government: nil)
       expect(edition.history_mode?).to be(false)
     end
 
     it "returns false when the edition is political and associated with the current government" do
-      edition = build(:edition, :political, :current_government)
+      edition = build(:edition, :political, government: current_government)
       expect(edition.history_mode?).to be(false)
     end
   end
 
   describe "#government" do
-    before { allow(Government).to receive(:all).and_return([government]) }
-    let(:government) { build(:government) }
-
     it "returns nil when government_id isn't set" do
       edition = build(:edition, government_id: nil)
       expect(edition.government).to be_nil
     end
 
     it "returns a government when one matches the id" do
+      government = build(:government)
+      populate_government_bulk_data(government)
+
       edition = build(:edition, government_id: government.content_id)
       expect(edition.government).to eq(government)
     end

From 5a5fc1a9ded8b75b6558a68fc3bc5f43798f1549 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 20:13:03 +0000
Subject: [PATCH 08/30] Use new government mocking approach for
 PreviewService::Payload

---
 spec/services/preview_service/payload_spec.rb | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/spec/services/preview_service/payload_spec.rb b/spec/services/preview_service/payload_spec.rb
index e86529534a..b56504512b 100644
--- a/spec/services/preview_service/payload_spec.rb
+++ b/spec/services/preview_service/payload_spec.rb
@@ -143,12 +143,14 @@
     end
 
     it "includes government when one is present" do
-      current_government = Government.current
-      edition = build(:edition, government_id: current_government.content_id)
+      government = build(:government)
+      populate_government_bulk_data(government)
+
+      edition = build(:edition, government_id: government.content_id)
 
       payload = PreviewService::Payload.new(edition).payload
 
-      expect(payload["links"]["government"]).to eq [current_government.content_id]
+      expect(payload["links"]["government"]).to eq [government.content_id]
     end
 
     it "includes a change note if the update type is 'major'" do

From 5aff300bebbdb2f0f68438dec59ba736ad1c4b94 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 21:13:36 +0000
Subject: [PATCH 09/30] Load governments from repository for PublishService

---
 app/services/publish_service.rb       |  5 +++--
 spec/services/publish_service_spec.rb | 30 +++++++++++++++++----------
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/app/services/publish_service.rb b/app/services/publish_service.rb
index ca0ac91bb3..d4b1a7f091 100644
--- a/app/services/publish_service.rb
+++ b/app/services/publish_service.rb
@@ -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)
 
diff --git a/spec/services/publish_service_spec.rb b/spec/services/publish_service_spec.rb
index a3886e7938..a224afee3a 100644
--- a/spec/services/publish_service_spec.rb
+++ b/spec/services/publish_service_spec.rb
@@ -6,6 +6,7 @@
 
     before do
       stub_any_publishing_api_publish
+      populate_default_government_bulk_data
       allow(PreviewService).to receive(:call)
       allow(PublishAssetService).to receive(:call)
     end
@@ -51,39 +52,46 @@
     end
 
     context "when the edition is not associated with a government" do
-      let(:government) { build(:government) }
+      let(:past_government) do
+        build(:government, ended_on: Time.zone.today)
+      end
+
+      let(:current_government) do
+        build(:government, started_on: Time.zone.today, current: true)
+      end
+
+      before do
+        populate_government_bulk_data(past_government, current_government)
+      end
 
       it "tries to associate a government with edition public first published at" do
         edition = create(:edition)
-        time = Time.zone.parse("2019-11-11")
-        allow(edition).to receive(:public_first_published_at).and_return(time)
-        expect(Government).to receive(:for_date).with(time)
-                          .and_return(government)
+        allow(edition).to receive(:public_first_published_at)
+                      .and_return(Time.zone.yesterday)
 
         expect { PublishService.call(edition, user, with_review: true) }
           .to change { edition.government_id }
-          .to(government.content_id)
+          .to(past_government.content_id)
       end
 
       it "associates with current government when the edition hasn't been published" do
         edition = create(:edition)
-        expect(Government).to receive(:current).and_return(government)
 
         expect { PublishService.call(edition, user, with_review: true) }
           .to change { edition.government_id }
-          .to(government.content_id)
+          .to(current_government.content_id)
       end
 
       it "updates the preview when a government is associated" do
         edition = create(:edition)
         expect(PreviewService).to receive(:call).with(edition)
         PublishService.call(edition, user, with_review: true)
-        expect(edition.government_id).to eq(Government.current.content_id)
+        expect(edition.government_id).to eq(current_government.content_id)
       end
 
       it "doesn't update the preview when a government isn't associated" do
+        populate_government_bulk_data
         edition = create(:edition)
-        allow(Government).to receive(:current).and_return(nil)
 
         expect(PreviewService).not_to receive(:call).with(edition)
         PublishService.call(edition, user, with_review: true)
@@ -92,7 +100,7 @@
     end
 
     context "when the edition is associated with a government" do
-      let(:edition) { create(:edition, :past_government) }
+      let(:edition) { create(:edition, government: past_government) }
 
       it "doesn't change the government on the edition" do
         expect { PublishService.call(edition, user, with_review: true) }

From 3bac6910ad9189e6740eb106e85a9041dd9e7557 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 21:37:20 +0000
Subject: [PATCH 10/30] Minor refactoring of document request spec

This removes unnecessary instance variables and reduces long lines, it
also removes an unnecessary test expectation.
---
 spec/requests/documents_spec.rb | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/spec/requests/documents_spec.rb b/spec/requests/documents_spec.rb
index 973c40515c..f6629c3639 100644
--- a/spec/requests/documents_spec.rb
+++ b/spec/requests/documents_spec.rb
@@ -5,25 +5,32 @@
     describe "history mode banner" do
       context "when in history mode" do
         it "shows the history mode banner" do
-          @edition = create(:edition, :political, :past_government)
-          get document_path(@edition.document)
-          expect(response.body).to include(I18n.t!("documents.show.historical.title", document_type: @edition.document_type.label.downcase))
-          expect(response.body).to include(I18n.t!("documents.show.historical.description", government_name: @edition.government.name))
+          edition = create(:edition, :political, :past_government)
+          get document_path(edition.document)
+          title = I18n.t!("documents.show.historical.title",
+                          document_type: edition.document_type.label.downcase)
+          description = I18n.t!("documents.show.historical.description",
+                                government_name: edition.government.name)
+          expect(response.body).to include(title)
+          expect(response.body).to include(description)
         end
 
         it "won't show the history mode banner when the edition was created under the current government" do
-          @edition = create(:edition, :political, :current_government)
-          get document_path(@edition.document)
-          expect(response.body).not_to include(I18n.t!("documents.show.historical.title", document_type: @edition.document_type.label.downcase))
-          expect(response.body).not_to include(I18n.t!("documents.show.historical.description", government_name: @edition.government.name))
+          edition = create(:edition, :political, :current_government)
+          get document_path(edition.document)
+          title = I18n.t!("documents.show.historical.title",
+                          document_type: edition.document_type.label.downcase)
+          expect(response.body).not_to include(title)
         end
       end
 
       context "when not in history_mode" do
         it "won't show the history mode banner" do
-          @edition = create(:edition, :not_political)
-          get document_path(@edition.document)
-          expect(response.body).not_to include(I18n.t!("documents.show.historical.title", document_type: @edition.document_type.label.downcase))
+          edition = create(:edition, :not_political)
+          get document_path(edition.document)
+          title = I18n.t!("documents.show.historical.title",
+                          document_type: edition.document_type.label.downcase)
+          expect(response.body).not_to include(title)
         end
       end
     end

From 49240a991822e464a068cd87f4ea6e0b05b90ed5 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 21:40:29 +0000
Subject: [PATCH 11/30] Fix incorrect I18n call

By using I18n.t we wouldn't be raising errors if a translation was
missing. We don't actually need to call I18n directly as Rails has a
view helper of t which roughly correlates to I18n.t!
---
 app/views/documents/show/_historical_notice.html.erb | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/app/views/documents/show/_historical_notice.html.erb b/app/views/documents/show/_historical_notice.html.erb
index d0e16fd93d..38c3e13f18 100644
--- a/app/views/documents/show/_historical_notice.html.erb
+++ b/app/views/documents/show/_historical_notice.html.erb
@@ -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.name),
   }
 %>

From dbe6b81509b321ae75f168c22a222d1f6b39122f Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 23:18:29 +0000
Subject: [PATCH 12/30] Use government repository in edit edition service

---
 app/services/edit_edition_service.rb       | 4 +++-
 spec/services/edit_edition_service_spec.rb | 9 ++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/app/services/edit_edition_service.rb b/app/services/edit_edition_service.rb
index 276a87d601..b61e8147b3 100644
--- a/app/services/edit_edition_service.rb
+++ b/app/services/edit_edition_service.rb
@@ -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
diff --git a/spec/services/edit_edition_service_spec.rb b/spec/services/edit_edition_service_spec.rb
index 16ea7576b6..2287a9d69d 100644
--- a/spec/services/edit_edition_service_spec.rb
+++ b/spec/services/edit_edition_service_spec.rb
@@ -64,10 +64,9 @@
     describe "associates the edition with a government" do
       it "sets the government when there is a public first published at time" do
         time = Time.zone.parse("2018-01-01")
-        government = build(:government)
-        edition = create(:edition)
+        government = build(:government, started_on: time)
+        populate_government_bulk_data(government)
         allow(edition).to receive(:public_first_published_at).and_return(time)
-        allow(Government).to receive(:for_date).with(time).and_return(government)
 
         expect { EditEditionService.call(edition, user) }
           .to change { edition.government_id }
@@ -76,8 +75,8 @@
 
       it "sets the government to nil when there isn't a government for the date" do
         publish_date = Time.zone.parse("2019-11-01")
-        allow(Government).to receive(:for_date).with(publish_date)
-                         .and_return(nil)
+        government = build(:government, started_on: publish_date.advance(days: 1))
+        populate_government_bulk_data(government)
         edition = build(:edition, first_published_at: publish_date)
 
         expect { EditEditionService.call(edition, user) }

From 59e57e43a93841402fbc2d992efc7979e3393278 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 23:22:43 +0000
Subject: [PATCH 13/30] Switch government name to title

This field is referred to as a title in the Publishing API
---
 app/views/documents/show/_historical_notice.html.erb | 2 +-
 spec/requests/documents_spec.rb                      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/views/documents/show/_historical_notice.html.erb b/app/views/documents/show/_historical_notice.html.erb
index 38c3e13f18..9e0506d2be 100644
--- a/app/views/documents/show/_historical_notice.html.erb
+++ b/app/views/documents/show/_historical_notice.html.erb
@@ -3,6 +3,6 @@
     title: t("documents.show.historical.title",
              document_type: @edition.document_type.label.downcase),
     description_text: t("documents.show.historical.description",
-                        government_name: @edition.government.name),
+                        government_name: @edition.government.title),
   }
 %>
diff --git a/spec/requests/documents_spec.rb b/spec/requests/documents_spec.rb
index f6629c3639..2d9609e3d9 100644
--- a/spec/requests/documents_spec.rb
+++ b/spec/requests/documents_spec.rb
@@ -10,7 +10,7 @@
           title = I18n.t!("documents.show.historical.title",
                           document_type: edition.document_type.label.downcase)
           description = I18n.t!("documents.show.historical.description",
-                                government_name: edition.government.name)
+                                government_name: edition.government.title)
           expect(response.body).to include(title)
           expect(response.body).to include(description)
         end

From 7ede62abeb567e1f3d96e9a577ae469a6b947494 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 23:23:47 +0000
Subject: [PATCH 14/30] Stub governments for tests

This sets up some default governments in advance of tests so that the
government data is populated. These are set-up before all feature and
request tests.
---
 spec/interactors/editions/create_interactor_spec.rb |  5 ++++-
 spec/jobs/scheduled_publishing_job_spec.rb          |  5 ++++-
 spec/spec_helper.rb                                 | 12 ++++++++----
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/spec/interactors/editions/create_interactor_spec.rb b/spec/interactors/editions/create_interactor_spec.rb
index 70dfdd8cbf..d32ddb889c 100644
--- a/spec/interactors/editions/create_interactor_spec.rb
+++ b/spec/interactors/editions/create_interactor_spec.rb
@@ -4,7 +4,10 @@
   describe ".call" do
     let(:user) { create :user }
 
-    before { stub_any_publishing_api_put_content }
+    before do
+      populate_default_government_bulk_data
+      stub_any_publishing_api_put_content
+    end
 
     it "resets the edition metadata" do
       edition = create(:edition,
diff --git a/spec/jobs/scheduled_publishing_job_spec.rb b/spec/jobs/scheduled_publishing_job_spec.rb
index 59cbd80510..3af4ff0a1e 100644
--- a/spec/jobs/scheduled_publishing_job_spec.rb
+++ b/spec/jobs/scheduled_publishing_job_spec.rb
@@ -3,7 +3,10 @@
 RSpec.describe ScheduledPublishingJob do
   include ActiveJob::TestHelper
 
-  before { stub_any_publishing_api_call }
+  before do
+    populate_default_government_bulk_data
+    stub_any_publishing_api_call
+  end
 
   let(:scheduled_edition) do
     scheduling = create(:scheduling, reviewed: true, publish_time: Time.current.yesterday)
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 1cc315a879..d57797e3b2 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -44,15 +44,19 @@
     Rails.application.load_seed
   end
 
+  config.before :each do
+    Sidekiq::Worker.clear_all
+    ActionMailer::Base.deliveries.clear
+    BulkData::Cache.clear
+  end
+
   config.before :each, type: :feature do
     # This is required by lots of specs when visiting the index page
     stub_publishing_api_has_linkables([], document_type: "organisation")
   end
 
-  config.before :each do
-    Sidekiq::Worker.clear_all
-    ActionMailer::Base.deliveries.clear
-    BulkData::Cache.clear
+  config.before :each, type: ->(spec) { spec.in?(%i[feature request]) } do
+    populate_default_government_bulk_data
   end
 
   config.after :each, type: ->(spec) { spec.in?(%i[feature request]) } do

From 5cc6f846500553cee836250ea1a843f24b009c58 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 9 Dec 2019 23:24:49 +0000
Subject: [PATCH 15/30] Re-populate governments when working with future dates

By jumping more than 24 hours into the future the cache entries for
governments expire, thus the government data needs to be re-stubbed.
---
 spec/features/scheduling/scheduled_publishing_failed_spec.rb     | 1 +
 spec/features/scheduling/scheduled_publishing_spec.rb            | 1 +
 .../scheduling/scheduled_publishing_without_review_spec.rb       | 1 +
 3 files changed, 3 insertions(+)

diff --git a/spec/features/scheduling/scheduled_publishing_failed_spec.rb b/spec/features/scheduling/scheduled_publishing_failed_spec.rb
index 77db995ebe..fee7a7d15e 100644
--- a/spec/features/scheduling/scheduled_publishing_failed_spec.rb
+++ b/spec/features/scheduling/scheduled_publishing_failed_spec.rb
@@ -53,6 +53,7 @@ def when_the_publishing_api_is_down
 
   def and_the_scheduled_publishing_job_runs
     travel_to(Time.zone.parse("2019-6-20 15:00"))
+    populate_default_government_bulk_data
     Sidekiq::Worker.drain_all
   end
 
diff --git a/spec/features/scheduling/scheduled_publishing_spec.rb b/spec/features/scheduling/scheduled_publishing_spec.rb
index 65c2afe198..2b49231176 100644
--- a/spec/features/scheduling/scheduled_publishing_spec.rb
+++ b/spec/features/scheduling/scheduled_publishing_spec.rb
@@ -64,6 +64,7 @@ def and_a_publish_intent_is_created
 
   def when_the_publishing_job_runs
     travel_to(Time.zone.parse("2019-6-22 9:00"))
+    populate_default_government_bulk_data
     stub_any_publishing_api_put_content
     @publish_request = stub_publishing_api_publish(@edition.content_id,
                                                    update_type: nil,
diff --git a/spec/features/scheduling/scheduled_publishing_without_review_spec.rb b/spec/features/scheduling/scheduled_publishing_without_review_spec.rb
index 34eb2a9ff4..53a6de8abc 100644
--- a/spec/features/scheduling/scheduled_publishing_without_review_spec.rb
+++ b/spec/features/scheduling/scheduled_publishing_without_review_spec.rb
@@ -48,6 +48,7 @@ def then_i_see_the_edition_is_scheduled
 
   def when_the_publishing_job_runs
     travel_to(Time.zone.parse("2019-8-10 12:00"))
+    populate_default_government_bulk_data
     stub_any_publishing_api_put_content
     @publish_request = stub_publishing_api_publish(@edition.content_id,
                                                    update_type: nil,

From 06d58aeff1f5d00a9c8ca535c8448755d0997b04 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Thu, 12 Dec 2019 20:36:51 +0000
Subject: [PATCH 16/30] Adjust request/feature tests for change in government
 trait

As we no longer have past_government and current_government traits this
injects these object in instead.
---
 spec/features/finding/index_filtering_spec.rb | 2 +-
 spec/requests/documents_spec.rb               | 4 ++--
 spec/requests/editions_spec.rb                | 3 +--
 spec/requests/unwithdraw_spec.rb              | 4 ++--
 spec/requests/withdraw_spec.rb                | 4 ++--
 5 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/spec/features/finding/index_filtering_spec.rb b/spec/features/finding/index_filtering_spec.rb
index 6cc60fd317..d6020c5741 100644
--- a/spec/features/finding/index_filtering_spec.rb
+++ b/spec/features/finding/index_filtering_spec.rb
@@ -44,7 +44,7 @@ def given_there_are_some_editions
 
     @relevant_edition = create(:edition,
                                :political,
-                               :past_government,
+                               government: past_government,
                                title: "Super relevant",
                                tags: {
                                  primary_publishing_organisation: [@primary_organisation["content_id"]],
diff --git a/spec/requests/documents_spec.rb b/spec/requests/documents_spec.rb
index 2d9609e3d9..cb1da5e85c 100644
--- a/spec/requests/documents_spec.rb
+++ b/spec/requests/documents_spec.rb
@@ -5,7 +5,7 @@
     describe "history mode banner" do
       context "when in history mode" do
         it "shows the history mode banner" do
-          edition = create(:edition, :political, :past_government)
+          edition = create(:edition, :political, government: past_government)
           get document_path(edition.document)
           title = I18n.t!("documents.show.historical.title",
                           document_type: edition.document_type.label.downcase)
@@ -16,7 +16,7 @@
         end
 
         it "won't show the history mode banner when the edition was created under the current government" do
-          edition = create(:edition, :political, :current_government)
+          edition = create(:edition, :political, government: current_government)
           get document_path(edition.document)
           title = I18n.t!("documents.show.historical.title",
                           document_type: edition.document_type.label.downcase)
diff --git a/spec/requests/editions_spec.rb b/spec/requests/editions_spec.rb
index bb0e21a484..a233f2415c 100644
--- a/spec/requests/editions_spec.rb
+++ b/spec/requests/editions_spec.rb
@@ -11,7 +11,7 @@
     end
 
     context "when the edition is in history mode" do
-      let(:edition) { create(:edition, :published, :political, :past_government) }
+      let(:edition) { create(:edition, :published, :political, government: past_government) }
 
       it "lets users holding the manage_live_history_mode permisssion create a new edition" do
         user = create(:user, manage_live_history_mode: true)
@@ -23,7 +23,6 @@
       end
 
       it "prevents users without the permission creating a new edition" do
-        edition = create(:edition, :published, :political, :past_government)
         post create_edition_path(edition.document)
 
         expect(response.body).to include(I18n.t!("missing_permissions.update_history_mode.title", title: edition.title))
diff --git a/spec/requests/unwithdraw_spec.rb b/spec/requests/unwithdraw_spec.rb
index 2d6365e03b..dc1d0306b5 100644
--- a/spec/requests/unwithdraw_spec.rb
+++ b/spec/requests/unwithdraw_spec.rb
@@ -33,7 +33,7 @@
     end
 
     context "when the edition is in history mode" do
-      let(:withdrawn_history_mode_edition) { create(:edition, :withdrawn, :political, :past_government) }
+      let(:withdrawn_history_mode_edition) { create(:edition, :withdrawn, :political, government: past_government) }
 
       it "lets users holding manage_live_history_mode permission unwithdraw the edition" do
         stub_publishing_api_republish(withdrawn_history_mode_edition.content_id, {})
@@ -87,7 +87,7 @@
     end
 
     context "when the edition is in history mode" do
-      let(:withdrawn_history_mode_edition) { create(:edition, :withdrawn, :political, :past_government) }
+      let(:withdrawn_history_mode_edition) { create(:edition, :withdrawn, :political, government: past_government) }
 
       it "lets managing_editors holding manage_live_history_mode permission to access unwithdraw page" do
         user = create(:user, managing_editor: true, manage_live_history_mode: true)
diff --git a/spec/requests/withdraw_spec.rb b/spec/requests/withdraw_spec.rb
index 6ebf5679d4..eb9592c075 100644
--- a/spec/requests/withdraw_spec.rb
+++ b/spec/requests/withdraw_spec.rb
@@ -45,7 +45,7 @@
     end
 
     context "when the edition is in history mode" do
-      let(:published_history_mode_edition) { create(:edition, :published, :political, :past_government) }
+      let(:published_history_mode_edition) { create(:edition, :published, :political, government: past_government) }
 
       it "lets managing_editors holding manage_live_history_mode permission withdraw the edition" do
         stub_publishing_api_unpublish(published_history_mode_edition.content_id, body: {})
@@ -103,7 +103,7 @@
     end
 
     context "when the edition is in history mode" do
-      let(:published_history_mode_edition) { create(:edition, :published, :political, :past_government) }
+      let(:published_history_mode_edition) { create(:edition, :published, :political, government: past_government) }
 
       it "lets managing_editors holding manage_live_history_mode permission to access withdraw page" do
         stub_publishing_api_unpublish(published_history_mode_edition.content_id, body: {})

From edcf5453a328c94133596d891a1186fa4c23362e Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Tue, 10 Dec 2019 00:08:29 +0000
Subject: [PATCH 17/30] Helper method to run ActiveJobs exclusively

This cuts back on the boilerplate needed to run a worker job on a single
box and deny another boxes that try and run this at the same time. This
is a situation that occurs with the sidekiq scheduler where every box
with a sidekiq process tries to run the job at the same time and the
first one to start it wins.

We counter this with a PostgreSQL advisory lock which prevents any other
clients using the db while this lock is held (they gracefully exit).
Ideally we want to run this lock inside a transaction so that if
anything goes wrong the lock can be automatically closed, whereas if we
run it outside a transaction and something crashes we may need to
manually release the lock.
---
 app/jobs/application_job.rb       | 18 +++++++
 app/jobs/asset_cleanup_job.rb     |  2 +-
 spec/jobs/application_job_spec.rb | 82 +++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 spec/jobs/application_job_spec.rb

diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb
index a8efeff0fa..7d72a977ab 100644
--- a/app/jobs/application_job.rb
+++ b/app/jobs/application_job.rb
@@ -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}"
+    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
diff --git a/app/jobs/asset_cleanup_job.rb b/app/jobs/asset_cleanup_job.rb
index e459a2bae9..4094685b97 100644
--- a/app/jobs/asset_cleanup_job.rb
+++ b/app/jobs/asset_cleanup_job.rb
@@ -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
diff --git a/spec/jobs/application_job_spec.rb b/spec/jobs/application_job_spec.rb
new file mode 100644
index 0000000000..792c053e8a
--- /dev/null
+++ b/spec/jobs/application_job_spec.rb
@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+
+require "with_advisory_lock/base"
+
+RSpec.describe ApplicationJob do
+  describe "#run_exclusively" do
+    let(:result) { WithAdvisoryLock::Result.new(true) }
+
+    it "runs the block within an advisory lock with a 0 timeout" do
+      code_block = double
+      expect(code_block).to receive(:run)
+
+      expect(ApplicationRecord)
+        .to receive(:with_advisory_lock_result)
+        .with(instance_of(String), a_hash_including(timeout_seconds: 0))
+        .and_yield
+        .and_return(result)
+
+      ApplicationJob.new.run_exclusively { code_block.run }
+    end
+
+    it "returns the result of the block" do
+      id = SecureRandom.uuid
+      returned_result = ApplicationJob.new.run_exclusively { id }
+      expect(returned_result).to eq(id)
+    end
+
+    it "logs when a lock can't be acquired" do
+      job = ApplicationJob.new
+      expect(ApplicationRecord)
+        .to receive(:with_advisory_lock_result)
+        .and_return(WithAdvisoryLock::Result.new(false))
+
+      expect(job.logger)
+        .to receive(:info)
+        .with("Job skipped as exclusive lock 'content-publisher:ApplicationJob' could not be acuqired")
+
+      job.run_exclusively
+    end
+
+    it "defaults to running within a transaction" do
+      expect(ApplicationRecord).to receive(:transaction).and_yield
+      expect(ApplicationRecord)
+        .to receive(:with_advisory_lock_result)
+        .with(instance_of(String), a_hash_including(transaction: true))
+        .and_return(result)
+
+      ApplicationJob.new.run_exclusively
+    end
+
+    it "can be run outside a transaction" do
+      expect(ApplicationRecord).not_to receive(:transaction)
+      expect(ApplicationRecord)
+        .to receive(:with_advisory_lock_result)
+        .with(instance_of(String), a_hash_including(transaction: false))
+        .and_return(result)
+
+      ApplicationJob.new.run_exclusively(transaction: false)
+    end
+
+    it "defaults to using the class name for the lock name" do
+      klass = Class.new(ApplicationJob)
+      allow(klass).to receive(:name).and_return("MyJob")
+
+      expect(ApplicationRecord)
+        .to receive(:with_advisory_lock_result)
+        .with("content-publisher:MyJob", instance_of(Hash))
+        .and_return(result)
+
+      klass.new.run_exclusively
+    end
+
+    it "can accept the lock name as an argument" do
+      expect(ApplicationRecord)
+        .to receive(:with_advisory_lock_result)
+        .with("content-publisher:lock-name", instance_of(Hash))
+        .and_return(result)
+
+      ApplicationJob.new.run_exclusively(lock_name: "lock-name")
+    end
+  end
+end

From c59ee819be7f65f9013b6335606a20bb4a2ffe22 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Tue, 10 Dec 2019 00:26:09 +0000
Subject: [PATCH 18/30] Default to fake sidekiq testing

It was more common that we were using Sidekiq::Testing.fake! than
relying on Sidekiq::Testing.inline! thus switching to this saves us
code. This is also a preferred approach to take since it keeps us more
honest by having to explicitly tell Sidekiq to perform actions rather
than expecting them to just happen synchronously.
---
 .../scheduled_publishing_failed_spec.rb       |  6 +-----
 .../scheduling/scheduled_publishing_spec.rb   |  6 +-----
 ...cheduled_publishing_without_review_spec.rb |  6 +-----
 .../scheduling/update_publish_time_spec.rb    |  4 +---
 spec/features/workflow/publish_spec.rb        | 19 ++++++++++---------
 .../workflow/publish_without_review_spec.rb   |  1 +
 spec/services/schedule_service_spec.rb        |  4 ----
 spec/spec_helper.rb                           |  2 +-
 8 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/spec/features/scheduling/scheduled_publishing_failed_spec.rb b/spec/features/scheduling/scheduled_publishing_failed_spec.rb
index fee7a7d15e..4307f04cb4 100644
--- a/spec/features/scheduling/scheduled_publishing_failed_spec.rb
+++ b/spec/features/scheduling/scheduled_publishing_failed_spec.rb
@@ -2,11 +2,7 @@
 
 RSpec.feature "Scheduled publishing failed" do
   around do |example|
-    Sidekiq::Testing.fake! do
-      travel_to(Time.zone.parse("2019-06-19"))
-      example.run
-      travel_back
-    end
+    travel_to(Time.zone.parse("2019-06-19")) { example.run }
   end
 
   scenario do
diff --git a/spec/features/scheduling/scheduled_publishing_spec.rb b/spec/features/scheduling/scheduled_publishing_spec.rb
index 2b49231176..7686438af6 100644
--- a/spec/features/scheduling/scheduled_publishing_spec.rb
+++ b/spec/features/scheduling/scheduled_publishing_spec.rb
@@ -2,11 +2,7 @@
 
 RSpec.feature "Scheduled publishing" do
   around do |example|
-    Sidekiq::Testing.fake! do
-      travel_to(Time.zone.parse("2019-06-21"))
-      example.run
-      travel_back
-    end
+    travel_to(Time.zone.parse("2019-06-21")) { example.run }
   end
 
   scenario do
diff --git a/spec/features/scheduling/scheduled_publishing_without_review_spec.rb b/spec/features/scheduling/scheduled_publishing_without_review_spec.rb
index 53a6de8abc..1aeab284c4 100644
--- a/spec/features/scheduling/scheduled_publishing_without_review_spec.rb
+++ b/spec/features/scheduling/scheduled_publishing_without_review_spec.rb
@@ -2,11 +2,7 @@
 
 RSpec.feature "Scheduled publishing without review" do
   around do |example|
-    Sidekiq::Testing.fake! do
-      travel_to(Time.zone.parse("2019-06-20"))
-      example.run
-      travel_back
-    end
+    travel_to(Time.zone.parse("2019-06-20")) { example.run }
   end
 
   scenario do
diff --git a/spec/features/scheduling/update_publish_time_spec.rb b/spec/features/scheduling/update_publish_time_spec.rb
index d1f3df0f5d..72d173eaa4 100644
--- a/spec/features/scheduling/update_publish_time_spec.rb
+++ b/spec/features/scheduling/update_publish_time_spec.rb
@@ -4,9 +4,7 @@
   include ActiveJob::TestHelper
 
   around do |example|
-    Sidekiq::Testing.fake! do
-      travel_to(Time.zone.parse("2019-06-13")) { example.run }
-    end
+    travel_to(Time.zone.parse("2019-06-13")) { example.run }
   end
 
   scenario do
diff --git a/spec/features/workflow/publish_spec.rb b/spec/features/workflow/publish_spec.rb
index 1f2d47ad3a..abfce59423 100644
--- a/spec/features/workflow/publish_spec.rb
+++ b/spec/features/workflow/publish_spec.rb
@@ -8,8 +8,8 @@
     then_i_see_the_publish_succeeded
     and_the_content_is_shown_as_published
     and_there_is_a_history_entry
-    and_i_receive_a_confirmation_email
     and_i_see_a_link_to_the_content_data_page_for_the_document
+    and_i_receive_a_confirmation_email
   end
 
   def given_there_is_a_major_change_to_a_live_edition
@@ -63,7 +63,16 @@ def and_there_is_a_history_entry
     expect(page).to have_content(I18n.t!("documents.history.entry_types.published"))
   end
 
+  def and_i_see_a_link_to_the_content_data_page_for_the_document
+    content_data_url_prefix = "https://content-data.test.gov.uk/metrics"
+    expect(page).to have_link(
+      "View data about this page",
+      href: content_data_url_prefix + @edition.base_path,
+    )
+  end
+
   def and_i_receive_a_confirmation_email
+    Sidekiq::Worker.drain_all
     tos = ActionMailer::Base.deliveries.map(&:to)
     message = ActionMailer::Base.deliveries.first
 
@@ -83,12 +92,4 @@ def and_i_receive_a_confirmation_email
                                            date: publish_date,
                                            user: publish_user))
   end
-
-  def and_i_see_a_link_to_the_content_data_page_for_the_document
-    content_data_url_prefix = "https://content-data.test.gov.uk/metrics"
-    expect(page).to have_link(
-      "View data about this page",
-      href: content_data_url_prefix + @edition.base_path,
-    )
-  end
 end
diff --git a/spec/features/workflow/publish_without_review_spec.rb b/spec/features/workflow/publish_without_review_spec.rb
index 95b0770303..ff7e8a926a 100644
--- a/spec/features/workflow/publish_without_review_spec.rb
+++ b/spec/features/workflow/publish_without_review_spec.rb
@@ -66,6 +66,7 @@ def then_i_see_that_its_reviewed
   end
 
   def and_the_editors_receive_an_email
+    Sidekiq::Worker.drain_all
     tos = ActionMailer::Base.deliveries.map(&:to)
     message = ActionMailer::Base.deliveries.first
 
diff --git a/spec/services/schedule_service_spec.rb b/spec/services/schedule_service_spec.rb
index 4fdbc0f022..7e5e9e5ed5 100644
--- a/spec/services/schedule_service_spec.rb
+++ b/spec/services/schedule_service_spec.rb
@@ -7,10 +7,6 @@
 
   include ActiveJob::TestHelper
 
-  around do |example|
-    Sidekiq::Testing.fake! { example.run }
-  end
-
   before(:each) do
     stub_default_publishing_api_put_intent
     allow(ScheduleService::Payload).to receive(:new) { payload }
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index d57797e3b2..c657cb6639 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -23,7 +23,7 @@
 Capybara.automatic_label_click = true
 ActiveRecord::Migration.maintain_test_schema!
 Rails.application.load_tasks
-Sidekiq::Testing.inline!
+Sidekiq::Testing.fake!
 
 RSpec.configure do |config|
   config.expose_dsl_globally = false

From 070b673f58560dd6207c2e2cb43e9b16547ea791 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Tue, 10 Dec 2019 20:42:55 +0000
Subject: [PATCH 19/30] Stub government data for edition filter specs

---
 spec/lib/edition_filter_spec.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/spec/lib/edition_filter_spec.rb b/spec/lib/edition_filter_spec.rb
index c6210e6b12..38a755def2 100644
--- a/spec/lib/edition_filter_spec.rb
+++ b/spec/lib/edition_filter_spec.rb
@@ -95,8 +95,10 @@
     end
 
     it "filters the editions that are in history mode" do
-      edition1 = create(:edition, :political, :current_government)
-      edition2 = create(:edition, :political, :past_government)
+      populate_default_government_bulk_data
+
+      edition1 = create(:edition, :political, government: current_government)
+      edition2 = create(:edition, :political, government: past_government)
 
       editions = EditionFilter.new(user, filters: { in_history_mode: "yes" }).editions
       expect(editions).to match_array([edition2])

From a7fd0d46722364814a5a27906ddf0970ccb6a063 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Tue, 10 Dec 2019 21:10:28 +0000
Subject: [PATCH 20/30] Add document live trait that sets first_published_at

The first_published_at value should always be set on documents that have
a live edition.

I didn't know a nice way to specify an optional trait to the association
method so have ended up with this array monstrosity. If you know a
better technique feel free to change it.
---
 spec/factories/document_factory.rb |  8 ++++++++
 spec/factories/edition_factory.rb  | 14 +++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/spec/factories/document_factory.rb b/spec/factories/document_factory.rb
index 5d770d0745..dab84f8d74 100644
--- a/spec/factories/document_factory.rb
+++ b/spec/factories/document_factory.rb
@@ -6,7 +6,13 @@
     locale { I18n.available_locales.sample }
     association :created_by, factory: :user
 
+    trait :live do
+      first_published_at { Time.current }
+    end
+
     trait :with_live_edition do
+      live
+
       after(:build) do |document, evaluator|
         document.live_edition = evaluator.association(
           :edition,
@@ -29,6 +35,8 @@
     end
 
     trait :with_current_and_live_editions do
+      live
+
       after(:build) do |document, evaluator|
         document.live_edition = evaluator.association(
           :edition,
diff --git a/spec/factories/edition_factory.rb b/spec/factories/edition_factory.rb
index 6be199f957..8a165e9137 100644
--- a/spec/factories/edition_factory.rb
+++ b/spec/factories/edition_factory.rb
@@ -25,13 +25,13 @@
 
     after(:build) do |edition, evaluator|
       unless edition.document
-        edition.document = evaluator.association(
-          :document,
-          created_by: edition.created_by,
-          content_id: evaluator.content_id,
-          locale: evaluator.locale,
-          first_published_at: evaluator.first_published_at,
-        )
+        args = [:document,
+                evaluator.live ? :live : nil,
+                created_by: edition.created_by,
+                content_id: evaluator.content_id,
+                locale: evaluator.locale,
+                first_published_at: evaluator.first_published_at]
+        edition.document = evaluator.association(*args.compact)
       end
 
       edition.number = edition.document&.next_edition_number unless edition.number

From 4f42b1bf7af272eb2351050e29414263f269801e Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Tue, 10 Dec 2019 21:59:02 +0000
Subject: [PATCH 21/30] Use government repository for ResyncService

---
 app/services/resync_service.rb       | 5 ++++-
 spec/services/resync_service_spec.rb | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/services/resync_service.rb b/app/services/resync_service.rb
index 2b185bc8a4..69e1f8bdaa 100644
--- a/app/services/resync_service.rb
+++ b/app/services/resync_service.rb
@@ -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
 
diff --git a/spec/services/resync_service_spec.rb b/spec/services/resync_service_spec.rb
index c133416c87..67bcae06fe 100644
--- a/spec/services/resync_service_spec.rb
+++ b/spec/services/resync_service_spec.rb
@@ -9,6 +9,7 @@
       stub_any_publishing_api_put_content
       stub_default_publishing_api_put_intent
       stub_default_publishing_api_path_reservation
+      populate_default_government_bulk_data
     end
 
     context "when there is no live edition" do
@@ -173,7 +174,7 @@
       let(:government) { build(:government) }
 
       before do
-        allow(Government).to receive(:all).and_return([government])
+        populate_government_bulk_data(government)
         allow(PoliticalEditionIdentifier)
           .to receive(:new)
           .and_return(instance_double(PoliticalEditionIdentifier, political?: true))

From 3d3245c3f6762bafea674c6e9d683fd7f873e70f Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Tue, 10 Dec 2019 22:19:55 +0000
Subject: [PATCH 22/30] Minor refactoring of ResyncService specs

This moves the it block outside of a context to be before context as per
the more common convention. It also removes the government and political
tests from their draft and live context since they aren't resultant
effects of that particular context.

Finally it makes the draft and live tests more consistent with the other
edition state tests by creating an edition rather than a document by
factory bot.
---
 spec/services/resync_service_spec.rb | 118 ++++++++++++++-------------
 1 file changed, 62 insertions(+), 56 deletions(-)

diff --git a/spec/services/resync_service_spec.rb b/spec/services/resync_service_spec.rb
index 67bcae06fe..435cf4bdd4 100644
--- a/spec/services/resync_service_spec.rb
+++ b/spec/services/resync_service_spec.rb
@@ -12,47 +12,90 @@
       populate_default_government_bulk_data
     end
 
+    it "reserves base paths" do
+      document = create(:document, :with_current_and_live_editions)
+
+      reserve_path_params = {
+        publishing_app: "content-publisher",
+        override_existing: true,
+      }
+
+      draft_request = stub_publishing_api_path_reservation(
+        document.current_edition.base_path,
+        reserve_path_params,
+      )
+      live_request = stub_publishing_api_path_reservation(
+        document.live_edition.base_path,
+        reserve_path_params,
+      )
+
+      ResyncService.call(document)
+
+      expect(draft_request).to have_been_requested
+      expect(live_request).to have_been_requested
+    end
+
+    it "updates the system_political value of editions" do
+      document = create(:document, :with_current_and_live_editions)
+
+      expect(PoliticalEditionIdentifier)
+        .to receive(:new)
+        .twice
+        .and_return(instance_double(PoliticalEditionIdentifier, political?: true))
+
+      expect { ResyncService.call(document) }
+        .to change { document.live_edition.system_political }.to(true)
+        .and change { document.current_edition.system_political }.to(true)
+    end
+
+    it "updates the government_id of editions" do
+      document = create(:document, :with_current_and_live_editions)
+      government = build(:government)
+      populate_government_bulk_data(government)
+
+      expect { ResyncService.call(document) }
+        .to change { document.live_edition.government_id }.to(government.content_id)
+        .and change { document.current_edition.government_id }.to(government.content_id)
+    end
+
     context "when there is no live edition" do
-      let(:document) { create(:document, :with_current_edition) }
+      let(:edition) { create(:edition) }
 
-      it "it does not publish the edition" do
-        expect(FailsafePreviewService).to receive(:call).with(document.current_edition)
+      it "doesn't publish the edition" do
+        expect(FailsafePreviewService).to receive(:call).with(edition)
         expect(GdsApi.publishing_api_v2).not_to receive(:publish)
-        ResyncService.call(document)
+        ResyncService.call(edition.document)
       end
     end
 
     context "when the current edition is live" do
-      let(:document) { create(:document, :with_live_edition) }
+      let(:edition) { create(:edition, :published) }
 
       it "avoids synchronising the edition twice" do
         expect(PreviewService).to receive(:call).once
-        ResyncService.call(document)
+        ResyncService.call(edition.document)
       end
 
       it "re-publishes the live edition" do
-        expect(PreviewService)
-          .to receive(:call)
-          .with(
-            document.current_edition,
-            republish: true,
-          )
-          .and_call_original
+        expect(PreviewService).to receive(:call)
+                              .with(edition, republish: true)
+                              .and_call_original
 
         request = stub_publishing_api_publish(
-          document.content_id,
+          edition.content_id,
           update_type: nil,
-          locale: document.locale,
+          locale: edition.locale,
         )
-        ResyncService.call(document)
+        ResyncService.call(edition.document)
 
         expect(request).to have_been_requested
       end
 
       it "publishes assets to the live stack" do
-        expect(PublishAssetService).to receive(:call).once.
-          with(document.live_edition, nil)
-        ResyncService.call(document)
+        expect(PublishAssetService)
+          .to receive(:call).once.with(edition, nil)
+
+        ResyncService.call(edition.document)
       end
     end
 
@@ -169,43 +212,6 @@
       end
     end
 
-    context "when there are both live and current editions" do
-      let(:document) { create(:document, :with_current_and_live_editions, first_published_at: Time.current) }
-      let(:government) { build(:government) }
-
-      before do
-        populate_government_bulk_data(government)
-        allow(PoliticalEditionIdentifier)
-          .to receive(:new)
-          .and_return(instance_double(PoliticalEditionIdentifier, political?: true))
-      end
-
-      it "updates the system_political value associated with both editions" do
-        expect { ResyncService.call(document) }
-          .to change { document.live_edition.system_political }.to(true)
-          .and change { document.current_edition.system_political }.to(true)
-      end
-
-      it "updates the government_id associated with with both editions" do
-        expect { ResyncService.call(document) }
-          .to change { document.live_edition.government_id }.to(government.content_id)
-          .and change { document.current_edition.government_id }.to(government.content_id)
-      end
-    end
-
-    it "reserves the path of the edition" do
-      edition = create(:edition)
-      reserve_path_params = {
-        publishing_app: "content-publisher",
-        override_existing: true,
-      }
-
-      request = stub_publishing_api_path_reservation(edition.base_path, reserve_path_params)
-      ResyncService.call(edition.document)
-
-      expect(request).to have_been_requested
-    end
-
     def stub_publishing_api_path_reservation(base_path, params)
       endpoint = GdsApi::TestHelpers::PublishingApi::PUBLISHING_API_ENDPOINT
       url = endpoint + "/paths#{base_path}"

From 69f496495342d3f17ecc49b01b5ee080910ff21d Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Tue, 10 Dec 2019 22:52:59 +0000
Subject: [PATCH 23/30] Add job to populate bulk data asynchronously

This sets up a job that is run every 15 minutes to update the bulk data.
If accessing the data fails this job is retried.

The schedule is set with an interval of 15 minutes and a delay of 0s which
makes the job start immediately whenever the worker process is restarted. As
this job is run on a schedule it will execute on all machines at the
same time. To counter this the job is run exclusively using the method
on the parent class.
---
 app/jobs/populate_bulk_data_job.rb       | 11 +++++++++
 config/sidekiq.yml                       |  5 ++++
 spec/jobs/populate_bulk_data_job_spec.rb | 31 ++++++++++++++++++++++++
 3 files changed, 47 insertions(+)
 create mode 100644 app/jobs/populate_bulk_data_job.rb
 create mode 100644 spec/jobs/populate_bulk_data_job_spec.rb

diff --git a/app/jobs/populate_bulk_data_job.rb b/app/jobs/populate_bulk_data_job.rb
new file mode 100644
index 0000000000..8063483985
--- /dev/null
+++ b/app/jobs/populate_bulk_data_job.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class PopulateBulkDataJob < ApplicationJob
+  retry_on BulkData::RemoteDataUnavailableError
+
+  def perform
+    run_exclusively do
+      BulkData::GovernmentRepository.new.populate_cache(older_than: 5.minutes.ago)
+    end
+  end
+end
diff --git a/config/sidekiq.yml b/config/sidekiq.yml
index 3c6305c707..62ffbd7a0d 100644
--- a/config/sidekiq.yml
+++ b/config/sidekiq.yml
@@ -12,3 +12,8 @@ development:
   asset_cleanup:
     interval: '30m'
     class: AssetCleanupJob
+  populate_bulk_data:
+    interval:
+      - 15m
+      - first_in: 0s
+    class: PopulateBulkDataJob
diff --git a/spec/jobs/populate_bulk_data_job_spec.rb b/spec/jobs/populate_bulk_data_job_spec.rb
new file mode 100644
index 0000000000..88e1f050a8
--- /dev/null
+++ b/spec/jobs/populate_bulk_data_job_spec.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+RSpec.describe PopulateBulkDataJob do
+  include ActiveJob::TestHelper
+
+  it "runs the job exclusively" do
+    job = PopulateBulkDataJob.new
+    expect(job).to receive(:run_exclusively)
+    job.perform
+  end
+
+  it "populates government caches older than 5 minutes" do
+    freeze_time do
+      repository = instance_double("BulkData::GovernmentRepository")
+      expect(BulkData::GovernmentRepository).to receive(:new).and_return(repository)
+      expect(repository).to receive(:populate_cache)
+                        .with(older_than: 5.minutes.ago)
+
+      PopulateBulkDataJob.perform_now
+    end
+  end
+
+  it "retries the job when the BulkData::RemoteDataUnavailableError is raised" do
+    expect(BulkData::GovernmentRepository)
+      .to receive(:new)
+      .and_raise(BulkData::RemoteDataUnavailableError)
+
+    PopulateBulkDataJob.perform_now
+    expect(PopulateBulkDataJob).to have_been_enqueued
+  end
+end

From dfb1eb22e45d7ee41ebc9a99cf10883c41fa0584 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Wed, 11 Dec 2019 12:04:37 +0000
Subject: [PATCH 24/30] Asynchronously load bulk data if it is not available

This will schedule a background job to load the data if for some reason
it's not available in the current environment and a service has tried to
use it.

There is a risk that this could cause a spike in identical queued jobs
were a bunch of users to get this at the same time. This shouldn't
present much of a problem as the job only executes one at a time and
won't do anything if the data was updated in the last 5 minutes.
---
 lib/bulk_data/government_repository.rb           | 1 +
 spec/lib/bulk_data/government_repository_spec.rb | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/bulk_data/government_repository.rb b/lib/bulk_data/government_repository.rb
index a6b5761f99..85307d13d2 100644
--- a/lib/bulk_data/government_repository.rb
+++ b/lib/bulk_data/government_repository.rb
@@ -26,6 +26,7 @@ def all
                     .map { |data| Government.new(data) }
                     .sort_by(&:started_on)
     rescue Cache::NoEntryError
+      PopulateBulkDataJob.perform_later
       raise LocalDataUnavailableError
     end
 
diff --git a/spec/lib/bulk_data/government_repository_spec.rb b/spec/lib/bulk_data/government_repository_spec.rb
index 8372795437..1df61e338c 100644
--- a/spec/lib/bulk_data/government_repository_spec.rb
+++ b/spec/lib/bulk_data/government_repository_spec.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 RSpec.describe BulkData::GovernmentRepository do
+  include ActiveJob::TestHelper
+
   let(:repository) { BulkData::GovernmentRepository.new }
   let(:cache_key) { BulkData::GovernmentRepository::CACHE_KEY }
 
@@ -99,9 +101,10 @@
       expect(repository.all).to eq([government_b, government_c, government_a])
     end
 
-    it "raises a LocalDataUnavailableError when there isn't data" do
+    it "raises an error and queues a job to populate date when there isn't data" do
       expect { repository.all }
         .to raise_error(BulkData::LocalDataUnavailableError)
+      expect(PopulateBulkDataJob).to have_been_enqueued
     end
   end
 

From b177dc22d08c3e6f5fe6630f6682efa7cd0abe5e Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Wed, 11 Dec 2019 13:04:08 +0000
Subject: [PATCH 25/30] Add an error view for
 BulkData::LocalDataUnavailableError

If the application is ever in a state where there is not local data
unavailable it will return a 503 response and a simple error message. As
I anticipate that developers may often be people who see this message
(at least while sidekiq isn't run by default with the app) I thought it
was useful to have explicit instructions in a developer environment.

It was pretty hard to think of the best way to test this so I went with
this rather contrived example since documents_show is one of the main
routes. I expect that when the BulkData technique is used as part of
index page filters we can replace this test with a less contrived one.
---
 app/controllers/application_controller.rb     |  8 +++++++
 .../errors/local_data_unavailable.html.erb    | 22 +++++++++++++++++++
 .../en/errors/local_data_unavailable.yml      |  8 +++++++
 spec/requests/errors_spec.rb                  | 14 ++++++++++++
 4 files changed, 52 insertions(+)
 create mode 100644 app/views/errors/local_data_unavailable.html.erb
 create mode 100644 config/locales/en/errors/local_data_unavailable.yml

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index a4de7a905d..9f3278cc3e 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -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
diff --git a/app/views/errors/local_data_unavailable.html.erb b/app/views/errors/local_data_unavailable.html.erb
new file mode 100644
index 0000000000..55fc2f35ec
--- /dev/null
+++ b/app/views/errors/local_data_unavailable.html.erb
@@ -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 %>
diff --git a/config/locales/en/errors/local_data_unavailable.yml b/config/locales/en/errors/local_data_unavailable.yml
new file mode 100644
index 0000000000..eade5bc837
--- /dev/null
+++ b/config/locales/en/errors/local_data_unavailable.yml
@@ -0,0 +1,8 @@
+en:
+  errors:
+    local_data_unavailable:
+      title: Sorry, there was a problem
+      body_govspeak: |
+        Please try again or [raise a support request][support-request].
+
+        [support-request]: https://support.publishing.service.gov.uk/technical_fault_report/new
diff --git a/spec/requests/errors_spec.rb b/spec/requests/errors_spec.rb
index db6a76bf39..e0b03dfd41 100644
--- a/spec/requests/errors_spec.rb
+++ b/spec/requests/errors_spec.rb
@@ -40,4 +40,18 @@
       expect(response).to have_http_status(:not_found)
     end
   end
+
+  describe "local data unavailable" do
+    it "returns service unavailable" do
+      # This is a somewhat contrived example, hopefully this can be replaced
+      # when there's a simple way to trigger this error
+      expect(Edition).to receive(:find_current)
+        .and_raise(BulkData::LocalDataUnavailableError)
+
+      get document_path("#{SecureRandom.uuid}:en")
+
+      expect(response).to have_http_status(:service_unavailable)
+      expect(response.body).to include(I18n.t!("errors.local_data_unavailable.title"))
+    end
+  end
 end

From d2ad3e76d595c6a6639574c4c666c97ded9c0380 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Wed, 11 Dec 2019 15:38:57 +0000
Subject: [PATCH 26/30] Remove hardcoded government config

This is no longer needed given we are now using the Publishing API
---
 config/governments.yml | 285 -----------------------------------------
 1 file changed, 285 deletions(-)
 delete mode 100644 config/governments.yml

diff --git a/config/governments.yml b/config/governments.yml
deleted file mode 100644
index ba7672dc08..0000000000
--- a/config/governments.yml
+++ /dev/null
@@ -1,285 +0,0 @@
-# This list of governments is generated from Whitehall by running the following
-# code in the Rails console:
-# `puts Government.order(start_date: :desc).map { |g| g.as_json(only: %i[content_id slug name start_date end_date]) }.to_yaml`
-# This is included until government data is in the Publishing API and will
-# need to be updating each time governments change.
-- slug: 2015-conservative-government
-  name: 2015 Conservative government
-  start_date: 2015-05-08
-  end_date:
-  content_id: d4fbc1b9-d47d-4386-af04-ac909f868f92
-- slug: 2010-to-2015-conservative-and-liberal-democrat-coalition-government
-  name: 2010 to 2015 Conservative and Liberal Democrat coalition government
-  start_date: 2010-05-12
-  end_date: 2015-05-08
-  content_id: 591c83aa-3b74-437d-a342-612bddd97572
-- slug: 2005-to-2010-labour-government
-  name: 2005 to 2010 Labour government
-  start_date: 2005-05-06
-  end_date: 2010-05-11
-  content_id: '09399599-397e-48c7-8bdb-171d6b49d387'
-- slug: 2001-to-2005-labour-government
-  name: 2001 to 2005 Labour government
-  start_date: 2001-06-08
-  end_date: 2005-05-05
-  content_id: 52f981c9-c73f-497b-aa91-86bd6092492b
-- slug: 1997-to-2001-labour-government
-  name: 1997 to 2001 Labour government
-  start_date: 1997-05-02
-  end_date: 2001-06-07
-  content_id: ee4de459-0941-40dd-ba36-f23eec0585cb
-- slug: 1992-to-1997-conservative-government
-  name: 1992 to 1997 Conservative government
-  start_date: 1992-04-10
-  end_date: 1997-05-01
-  content_id: a1d4477d-ad05-4e8c-9150-062765c901b8
-- slug: 1987-to-1992-conservative-government
-  name: 1987 to 1992 Conservative government
-  start_date: 1987-06-12
-  end_date: 1992-04-09
-  content_id: 6b79f257-8d65-44fa-a9e4-225a57fe7963
-- slug: 1983-to-1987-conservative-government
-  name: 1983 to 1987 Conservative government
-  start_date: 1983-06-10
-  end_date: 1987-06-11
-  content_id: f4f04740-e945-4be1-951e-ab0dab1ca905
-- slug: 1979-to-1983-conservative-government
-  name: 1979 to 1983 Conservative government
-  start_date: 1979-05-04
-  end_date: 1983-06-09
-  content_id: 2d8028c2-a2d1-417a-a115-745196fe17f8
-- slug: 1974-to-1979-labour-government
-  name: 1974 to 1979 Labour government
-  start_date: 1974-10-11
-  end_date: 1979-05-03
-  content_id: 2eb3350f-6a54-466b-9a52-07b15f6b3c0c
-- slug: 1974-to-1974-labour-minority-government
-  name: 1974 to 1974 Labour (minority) government
-  start_date: 1974-03-04
-  end_date: 1974-10-10
-  content_id: 88f8bf5b-6be4-4eb4-a2c9-48d1375c7671
-- slug: 1970-to-1974-conservative-government
-  name: 1970 to 1974 Conservative government
-  start_date: 1970-06-19
-  end_date: 1974-03-03
-  content_id: f3eaf7f0-00fc-4a04-a895-289e26dd0cff
-- slug: 1966-to-1970-labour-government
-  name: 1966 to 1970 Labour government
-  start_date: 1966-04-01
-  end_date: 1970-06-18
-  content_id: bba3f518-4d64-473d-9ff9-6a9f9c3f33c8
-- slug: 1964-to-1966-labour-government
-  name: 1964 to 1966 Labour government
-  start_date: 1964-10-16
-  end_date: 1966-03-31
-  content_id: 8efa5f78-be3f-48cc-8aab-61fbb78c256d
-- slug: 1959-to-1964-conservative-government
-  name: 1959 to 1964 Conservative government
-  start_date: 1959-10-09
-  end_date: 1964-10-15
-  content_id: 5405d71b-c07c-4f92-9dbc-3d0cc36066f4
-- slug: 1955-to-1959-conservative-government
-  name: 1955 to 1959 Conservative government
-  start_date: 1955-05-27
-  end_date: 1959-10-08
-  content_id: 7f62d7bc-a9f5-4073-b042-01537579036f
-- slug: 1951-to-1955-conservative-government
-  name: 1951 to 1955 Conservative government
-  start_date: 1951-10-26
-  end_date: 1955-05-26
-  content_id: 603fbd65-1300-49b4-a897-31336dea5297
-- slug: 1950-to-1951-labour-government
-  name: 1950 to 1951 Labour government
-  start_date: 1950-02-24
-  end_date: 1951-10-25
-  content_id: '098c877d-1f39-424f-b7cc-c04119b3c91e'
-- slug: 1945-to-1950-labour-government
-  name: 1945 to 1950 Labour government
-  start_date: 1945-07-27
-  end_date: 1950-02-23
-  content_id: e43512df-4830-45a8-b94d-d27f83ecead9
-- slug: 1935-to-1945-national-government
-  name: 1935 to 1945 National government
-  start_date: 1935-11-15
-  end_date: 1945-07-26
-  content_id: a05c2130-60c3-4d92-9f30-1cd3fb9d4a00
-- slug: 1931-to-1935-national-government
-  name: 1931 to 1935 National government
-  start_date: 1931-10-28
-  end_date: 1935-11-14
-  content_id: 7a893530-a089-4b43-ae14-7472dbe900a2
-- slug: 1929-to-1931-labour-minority-government
-  name: 1929 to 1931 Labour (minority) government
-  start_date: 1929-06-05
-  end_date: 1931-10-27
-  content_id: 18494aed-7b2e-4b3d-adc7-60c41a5df82a
-- slug: 1924-to-1929-conservative-government
-  name: 1924 to 1929 Conservative government
-  start_date: 1924-10-30
-  end_date: 1929-06-04
-  content_id: 1c33a365-3d9f-4da2-ac43-cfddb0fbb2d7
-- slug: 1924-to-1924-labour-minority-government
-  name: 1924 to 1924 Labour (minority) government
-  start_date: 1924-01-22
-  end_date: 1924-10-29
-  content_id: 7f7e85d5-487e-4961-ba0f-1b5b455b7d8f
-- slug: 1922-to-1924-conservative-government
-  name: 1922 to 1924 Conservative government
-  start_date: 1922-11-16
-  end_date: 1924-01-21
-  content_id: be5da979-b7d6-436e-afe4-45d9ea4773c9
-- slug: 1918-to-1922-conservative-and-liberal-coalition-government
-  name: 1918 to 1922 Conservative and Liberal coalition government
-  start_date: 1918-12-15
-  end_date: 1922-11-15
-  content_id: b8c94a77-1842-4d8a-9e1e-92847e2286da
-- slug: 1910-to-1918-liberal-minority-government
-  name: 1910 to 1918 Liberal (minority) government
-  start_date: 1910-12-19
-  end_date: 1918-12-14
-  content_id: 83478dbe-3141-4cb3-83b2-f55c16f478ca
-- slug: 1910-to-1910-liberal-minority-government
-  name: 1910 to 1910 Liberal (minority) government
-  start_date: 1910-02-10
-  end_date: 1910-12-18
-  content_id: cc329b72-09c4-489a-af5c-579a19d71c52
-- slug: 1906-to-1910-liberal-government
-  name: 1906 to 1910 Liberal government
-  start_date: 1906-02-09
-  end_date: 1910-02-09
-  content_id: af30abf2-2cf7-4487-90bd-b629464efa1e
-- slug: 1900-to-1906-conservative-government
-  name: 1900 to 1906 Conservative government
-  start_date: 1900-10-25
-  end_date: 1906-02-08
-  content_id: 4ba70052-dfa8-4d4c-9bb8-fc6904007aa1
-- slug: 1895-to-1900-conservative-government
-  name: 1895 to 1900 Conservative government
-  start_date: 1895-08-08
-  end_date: 1900-10-24
-  content_id: 3cc4a8da-535b-42bb-a861-33ecd98ce773
-- slug: 1892-to-1895-liberal-minority-government
-  name: 1892 to 1895 Liberal (minority) government
-  start_date: 1892-08-15
-  end_date: 1895-08-07
-  content_id: e3372321-5997-4324-a1e7-fedb4a31db30
-- slug: 1886-to-1892-conservative-government
-  name: 1886 to 1892 Conservative government
-  start_date: 1886-08-25
-  end_date: 1892-08-14
-  content_id: c024b258-d9a4-473e-b86e-19ea8ceaea38
-- slug: 1885-to-1886-liberal-minority-government
-  name: 1885 to 1886 Liberal (minority) government
-  start_date: 1885-12-19
-  end_date: 1886-08-24
-  content_id: 41cdb486-3bde-4bcd-aa24-dc0346b1b182
-- slug: 1880-to-1885-liberal-government
-  name: 1880 to 1885 Liberal government
-  start_date: 1880-04-23
-  end_date: 1885-12-18
-  content_id: 55c59c36-75f1-4b09-bb05-2daf0654865f
-- slug: 1874-to-1880-conservative-government
-  name: 1874 to 1880 Conservative government
-  start_date: 1874-02-18
-  end_date: 1880-04-22
-  content_id: dfa0162a-42ed-4948-8537-9e96b037a0e8
-- slug: 1868-to-1874-liberal-government
-  name: 1868 to 1874 Liberal government
-  start_date: 1868-12-08
-  end_date: 1874-02-17
-  content_id: 17eb7b78-f92d-4900-84c7-b1454b996698
-- slug: 1865-to-1868-liberal-government
-  name: 1865 to 1868 Liberal government
-  start_date: 1865-07-25
-  end_date: 1868-12-07
-  content_id: 6e95ac66-19d6-4f70-94a4-bdb2f3411bb8
-- slug: 1859-to-1865-liberal-government
-  name: 1859 to 1865 Liberal government
-  start_date: 1859-05-19
-  end_date: 1865-07-24
-  content_id: 3e886b7b-62fd-49f9-9737-ce8b7f942bc2
-- slug: 1857-to-1859-whig-government
-  name: 1857 to 1859 Whig government
-  start_date: 1857-04-25
-  end_date: 1859-05-18
-  content_id: 69b0f69a-2544-4a76-bc73-2d8673cd3423
-- slug: 1852-to-1857-conservative-government
-  name: 1852 to 1857 Conservative government
-  start_date: 1852-08-01
-  end_date: 1857-04-24
-  content_id: 36e0cc46-63c9-414c-9540-638d7d1fc6c9
-- slug: 1847-to-1852-whig-government
-  name: 1847 to 1852 Whig government
-  start_date: 1847-08-27
-  end_date: 1852-07-31
-  content_id: a1996829-eb94-469b-92fd-e59b35b20de2
-- slug: 1841-to-1847-conservative-government
-  name: 1841 to 1847 Conservative government
-  start_date: 1841-07-23
-  end_date: 1847-08-26
-  content_id: f9630eb9-1a76-45db-8f83-5db13ac00030
-- slug: 1837-to-1841-whig-government
-  name: 1837 to 1841 Whig government
-  start_date: 1837-08-19
-  end_date: 1841-07-22
-  content_id: e9b2f798-3679-4c25-8ce6-3d789c886681
-- slug: 1835-to-1837-whig-government
-  name: 1835 to 1837 Whig government
-  start_date: 1835-04-09
-  end_date: 1837-08-18
-  content_id: 129ef024-a03b-470e-b7c0-0ead709cdf28
-- slug: 1833-to-1835-whig-government
-  name: 1833 to 1835 Whig government
-  start_date: 1833-01-29
-  end_date: 1835-04-08
-  content_id: 4f5d93e2-e5d5-4a17-b376-c8a36ac71a01
-- slug: 1831-to-1833-whig-government
-  name: 1831 to 1833 Whig government
-  start_date: 1831-06-14
-  end_date: 1833-01-28
-  content_id: f615471f-3f77-407c-bade-9e0134c56bb7
-- slug: 1830-to-1831-whig-government
-  name: 1830 to 1831 Whig government
-  start_date: 1830-09-14
-  end_date: 1831-06-13
-  content_id: a085fb04-564b-4115-a37e-0ef7790093f0
-- slug: 1826-to-1830-tory-government
-  name: 1826 to 1830 Tory government
-  start_date: 1826-07-25
-  end_date: 1830-09-13
-  content_id: 35974059-379f-4b38-8ad3-e3b592fed14f
-- slug: 1820-to-1826-tory-government
-  name: 1820 to 1826 Tory government
-  start_date: 1820-04-21
-  end_date: 1826-07-24
-  content_id: 0a64be42-43fb-44dd-81b5-ca869aacf5b0
-- slug: 1818-to-1820-tory-government
-  name: 1818 to 1820 Tory government
-  start_date: 1818-08-04
-  end_date: 1820-04-20
-  content_id: d1c4eb6e-2551-42fb-8ba1-3b8d5ae659e5
-- slug: 1812-to-1818-tory-government
-  name: 1812 to 1818 Tory government
-  start_date: 1812-11-24
-  end_date: 1818-08-03
-  content_id: 9add4800-a469-4d07-91a0-09ca77c1cfe5
-- slug: 1807-to-1812-tory-government
-  name: 1807 to 1812 Tory government
-  start_date: 1807-06-22
-  end_date: 1812-11-23
-  content_id: c3a959ae-fed6-4a1a-a085-1006b591d136
-- slug: 1806-to-1807-whig-government
-  name: 1806 to 1807 Whig government
-  start_date: 1806-12-13
-  end_date: 1807-06-21
-  content_id: 83d0f35d-0e3e-437c-8fa1-4122326c7aad
-- slug: 1802-to-1806-tory-government
-  name: 1802 to 1806 Tory government
-  start_date: 1802-08-31
-  end_date: 1806-12-12
-  content_id: 79148554-b29d-4a85-aa37-a8c3da8346fd
-- slug: 1801-to-1802-tory-government
-  name: 1801 to 1802 Tory government
-  start_date: 1801-01-22
-  end_date: 1802-08-30
-  content_id: 04c65c7c-072d-48ef-8251-9f9dd7dc94cc#

From be7cb1f43c1edf5a1d2ce177cadf2f3052df3741 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Thu, 12 Dec 2019 12:38:33 +0000
Subject: [PATCH 27/30] Use specific government in history mode spec

This spec relies on a government existing at the date we specify which
caused this test to fail. By creating the government in the test we
resolve this complication.
---
 .../history_mode_spec.rb                      | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/spec/features/editing_content_settings/history_mode_spec.rb b/spec/features/editing_content_settings/history_mode_spec.rb
index 0bc6bbef21..321db750c3 100644
--- a/spec/features/editing_content_settings/history_mode_spec.rb
+++ b/spec/features/editing_content_settings/history_mode_spec.rb
@@ -2,7 +2,8 @@
 
 RSpec.feature "History mode" do
   scenario do
-    given_there_is_a_not_political_document
+    given_there_is_a_past_government
+    and_there_is_a_not_political_document
     and_i_am_a_managing_editor
     when_i_visit_the_summary_page
     then_i_see_that_the_content_doesnt_get_history_mode
@@ -17,7 +18,15 @@
     and_i_see_the_history_mode_banner
   end
 
-  def given_there_is_a_not_political_document
+  def given_there_is_a_past_government
+    @government = build(:government,
+                        started_on: Time.zone.parse("2006-01-01"),
+                        ended_on: Time.zone.parse("2010-01-01"))
+
+    populate_government_bulk_data(@government)
+  end
+
+  def and_there_is_a_not_political_document
     @edition = create(:edition, :not_political)
   end
 
@@ -69,9 +78,9 @@ def when_i_click_to_backdate_the_content
 
   def and_i_enter_a_date_to_backdate_the_content_to
     @request = stub_publishing_api_put_content(@edition.content_id, {})
-    fill_in "backdate[date][day]", with: "1"
-    fill_in "backdate[date][month]", with: "1"
-    fill_in "backdate[date][year]", with: "1999"
+    fill_in "backdate[date][day]", with: @government.started_on.day
+    fill_in "backdate[date][month]", with: @government.started_on.month
+    fill_in "backdate[date][year]", with: @government.started_on.year
     click_on "Save"
   end
 

From 8399dd5011ea4fd8a3b0e59d7dc263e9c2675c8f Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Thu, 12 Dec 2019 23:50:48 +0000
Subject: [PATCH 28/30] Use local cache Rack middleware with BulkData::Cache

This sets up a local cache that wraps around the Redis cache layer of
BulkData::Cache during the duration of a request.

In order for this to work in a development environment we need the
BulkData::Cache class not to be reloaded automatically. I achieved this
by doing direct require calls. I first tried using the Rails
autoload_once_paths option but didn't have much luck.

As an example, before fixed require calls:

```
irb(main):002:0> BulkData::Cache.object_id
=> 70342543814920
irb(main):003:0> reload!
Reloading...
=> true
irb(main):004:0> BulkData::Cache.object_id
=> 70342595490220
```

and after:

```
irb(main):002:0> BulkData::Cache.object_id
=> 70249028613320
irb(main):004:0> reload!
Reloading...
=> true
irb(main):005:0> BulkData::Cache.object_id
=> 70249028613320
```
---
 config/initializers/bulk_data_rack_middleware.rb | 14 ++++++++++++++
 lib/bulk_data/cache.rb                           |  3 +++
 2 files changed, 17 insertions(+)
 create mode 100644 config/initializers/bulk_data_rack_middleware.rb

diff --git a/config/initializers/bulk_data_rack_middleware.rb b/config/initializers/bulk_data_rack_middleware.rb
new file mode 100644
index 0000000000..10397735df
--- /dev/null
+++ b/config/initializers/bulk_data_rack_middleware.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+require "bulk_data/cache"
+
+# This is used to set a local cache that runs on BulkData for the duration of
+# a request. This means if we look up the same item in the cache multiple
+# times during a single request it will only hit Redis once and then look up
+# the item in memory.
+#
+# For more details see: https://github.com/rails/rails/blob/fa292703e1b733a7a55a8d6f0d749ddf590c61fd/activesupport/lib/active_support/cache/strategy/local_cache.rb
+Rails.application.config.middleware.insert_before(
+  ::Rack::Runtime,
+  BulkData::Cache.middleware,
+)
diff --git a/lib/bulk_data/cache.rb b/lib/bulk_data/cache.rb
index e39ba67e5f..b9b140f510 100644
--- a/lib/bulk_data/cache.rb
+++ b/lib/bulk_data/cache.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "bulk_data"
+
 module BulkData
   class Cache
     include Singleton
@@ -8,6 +10,7 @@ class NoEntryError < RuntimeError; end
 
     class << self
       delegate :cache, to: :instance
+      delegate :clear, :middleware, to: :cache
     end
 
     attr_reader :cache

From fa27e4c73458f9c19ebd014c778a122ee9458366 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 16 Dec 2019 09:29:30 +0000
Subject: [PATCH 29/30] Configure Redis for resiliency and reporting

By default the RedisCacheStore uses a fault tolerant approach where it
returns nil if it can't access Redis and writes to a logger that we
haven't configured [1].

This isn't an ideal configuration for our use case as it's a critical
problem if the cache isn't available. To try resolve this we've got 3
reconnect attempts available if we don't manage to connect to Redis or
have lost our connection.

In the event that we have an error reading from the cache this will
report the error to sentry, which will hopefully help make problems
easier to diagnose.

[1]: https://github.com/rails/rails/blob/09a2979f75c51afb797dd60261a8930f84144af8/activesupport/lib/active_support/cache/redis_cache_store.rb#L472-L485
---
 lib/bulk_data/cache.rb | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/bulk_data/cache.rb b/lib/bulk_data/cache.rb
index b9b140f510..65ea8dfe75 100644
--- a/lib/bulk_data/cache.rb
+++ b/lib/bulk_data/cache.rb
@@ -49,6 +49,9 @@ def self.clear
     def initialize
       @cache = ActiveSupport::Cache::RedisCacheStore.new(
         namespace: "content-publisher:bulk-data-cache-#{Rails.env}",
+        error_handler: ->(exception:, **) { GovukError.notify(exception) },
+        reconnect_attempts: 3,
+        reconnect_delay: 0.1,
       )
     end
   end

From d395aa5668b6c928401ce7adc09ccaca9324f522 Mon Sep 17 00:00:00 2001
From: Kevin Dew <kevindew@me.com>
Date: Mon, 16 Dec 2019 09:49:59 +0000
Subject: [PATCH 30/30] Don't report BulkData::RemoteDataUnavailableError on
 server errors

There's not a whole lot we can do when a dependent service is erroring
so lets not report it and wait to see if it causes problems in the app.

This is to resolve an issue where we start getting server errors
overnight in integration at the point of the data sync.
---
 app/jobs/populate_bulk_data_job.rb       |  4 +++-
 spec/jobs/populate_bulk_data_job_spec.rb | 21 +++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/app/jobs/populate_bulk_data_job.rb b/app/jobs/populate_bulk_data_job.rb
index 8063483985..bd802b7432 100644
--- a/app/jobs/populate_bulk_data_job.rb
+++ b/app/jobs/populate_bulk_data_job.rb
@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
 class PopulateBulkDataJob < ApplicationJob
-  retry_on BulkData::RemoteDataUnavailableError
+  retry_on BulkData::RemoteDataUnavailableError do |_job, error|
+    GovukError.notify(error) unless error.cause.is_a?(GdsApi::HTTPServerError)
+  end
 
   def perform
     run_exclusively do
diff --git a/spec/jobs/populate_bulk_data_job_spec.rb b/spec/jobs/populate_bulk_data_job_spec.rb
index 88e1f050a8..961c3ff762 100644
--- a/spec/jobs/populate_bulk_data_job_spec.rb
+++ b/spec/jobs/populate_bulk_data_job_spec.rb
@@ -28,4 +28,25 @@
     PopulateBulkDataJob.perform_now
     expect(PopulateBulkDataJob).to have_been_enqueued
   end
+
+  context "when it runs out of retries" do
+    it "reports the error to GovukError if the cause is not a GdsApi::HTTPServerError" do
+      stub_any_publishing_api_call.to_return(status: 429)
+
+      perform_enqueued_jobs do
+        expect(GovukError).to receive(:notify)
+                          .with(instance_of(BulkData::RemoteDataUnavailableError))
+        PopulateBulkDataJob.perform_later
+      end
+    end
+
+    it "doesn't report the error to GovukError when the cause is a GdsApi::HTTPServerError" do
+      stub_any_publishing_api_call.to_return(status: 500)
+
+      perform_enqueued_jobs do
+        expect(GovukError).not_to receive(:notify)
+        PopulateBulkDataJob.perform_later
+      end
+    end
+  end
 end