CLDC-4066: Fix uprn timeout to match address search

in practice waiting up to 1 min 30 for requests could cause too many threads to be blocked under periods of slow OS Places API

this reduces the max wait time to 45 seconds, as requested by CORE

Gemfile.lock

@@ -411,7 +411,7 @@ GEM
     responders (3.1.1)
       actionpack (>= 5.2)
       railties (>= 5.2)
-    rexml (3.3.9)
+    rexml (3.4.4)
     roo (2.10.1)
       nokogiri (~> 1)
       rubyzip (>= 1.3.0, < 3.0.0)

app/frontend/controllers/address_search_controller.js

@@ -4,10 +4,28 @@ import 'accessible-autocomplete/dist/accessible-autocomplete.min.css'
 
 const options = []
 
+let latestQueryId = 0
+
+const sleep = (ms) => {
+  return new Promise(resolve => setTimeout(resolve, ms))
+}
+
 const fetchOptions = async (query, searchUrl) => {
   if (query.length < 2) {
     throw new Error('Query must be at least 2 characters long.')
   }
+
+  // implement a debounce
+  // this is because this API has periods of high latency if OS Places has an outage
+  // making too many requests can overwhelm the number of threads available on the server
+  // which can in turn cause a site wide outage
+  latestQueryId++
+  const myQueryId = latestQueryId
+  await sleep(500)
+  if (myQueryId !== latestQueryId) {
+    throw new Error('Outdated query, ignoring result.')
+  }
+
   try {
     const response = await fetch(`${searchUrl}?query=${encodeURIComponent(query.trim())}`)
     return await response.json()

app/services/address_client.rb

@@ -35,7 +35,7 @@ private
     client.use_ssl = true
     client.verify_mode = OpenSSL::SSL::VERIFY_PEER
     client.max_retries = 3
-    client.read_timeout = 30 # seconds
+    client.read_timeout = 15 # seconds
     client
   end
 

app/services/exports/lettings_log_export_service.rb

@@ -30,14 +30,32 @@ module Exports
       "core_#{year}_#{year + 1}_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, year)
+    def retrieve_resources_from_range(range, year)
-      if !full_update && recent_export
+      relation = LettingsLog.exportable.filter_by_year(year).left_joins(:created_by, :updated_by, :assigned_to, :owning_organisation, :managing_organisation)
-        params = { from: recent_export.started_at, to: @start_time }
+
-        LettingsLog.exportable.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params).filter_by_year(year)
+      ids = relation
-      else
+        .where({ updated_at: range })
-        params = { to: @start_time }
+        .or(
-        LettingsLog.exportable.where("updated_at <= :to", params).filter_by_year(year)
+          relation.where.not(values_updated_at: nil).where(values_updated_at: range),
-      end
+        )
+        .or(
+          relation.where.not({ created_by: { updated_at: nil } }).where({ created_by: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ updated_by: { updated_at: nil } }).where({ updated_by: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ assigned_to: { updated_at: nil } }).where({ assigned_to: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ owning_organisation: { updated_at: nil } }).where({ owning_organisation: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ managing_organisation: { updated_at: nil } }).where({ managing_organisation: { updated_at: range } }),
+        )
+        .pluck(:id)
+
+      LettingsLog.where(id: ids)
     end
 
     def apply_cds_transformation(lettings_log, export_mode)

app/services/exports/organisation_export_service.rb

@@ -25,24 +25,19 @@ module Exports
       "organisations_2024_2025_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, _year)
+    def retrieve_resources_from_range(range, _year)
-      if !full_update && recent_export
+      relation = Organisation.left_joins(:users, :organisation_name_changes)
-        params = { from: recent_export.started_at, to: @start_time }
+      ids = relation
+              .where({ updated_at: range })
+              .or(
+                relation.where(organisation_name_changes: { created_at: range }),
+              )
+              .or(
+                relation.where(users: { is_dpo: true, updated_at: range }),
+              )
+              .pluck(:id)
 
-        Organisation
+      Organisation.where(id: ids)
-          .where(updated_at: params[:from]..params[:to])
-          .or(
-            Organisation.where(id: OrganisationNameChange.where(created_at: params[:from]..params[:to]).select(:organisation_id)),
-          )
-      else
-        params = { to: @start_time }
-
-        Organisation
-          .where("updated_at <= :to", params)
-          .or(
-            Organisation.where(id: OrganisationNameChange.where("created_at <= :to", params).select(:organisation_id)),
-          )
-      end
     end
 
     def build_export_xml(organisations)

app/services/exports/sales_log_export_service.rb

@@ -30,14 +30,32 @@ module Exports
       "core_sales_#{year}_#{year + 1}_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, year)
+    def retrieve_resources_from_range(range, year)
-      if !full_update && recent_export
+      relation = SalesLog.exportable.filter_by_year(year).left_joins(:created_by, :updated_by, :assigned_to, :owning_organisation, :managing_organisation)
-        params = { from: recent_export.started_at, to: @start_time }
+
-        SalesLog.exportable.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params).filter_by_year(year)
+      ids = relation
-      else
+              .where({ updated_at: range })
-        params = { to: @start_time }
+              .or(
-        SalesLog.exportable.where("updated_at <= :to", params).filter_by_year(year)
+                relation.where.not(values_updated_at: nil).where(values_updated_at: range),
-      end
+              )
+              .or(
+                relation.where.not({ created_by: { updated_at: nil } }).where({ created_by: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ updated_by: { updated_at: nil } }).where({ updated_by: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ assigned_to: { updated_at: nil } }).where({ assigned_to: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ owning_organisation: { updated_at: nil } }).where({ owning_organisation: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ managing_organisation: { updated_at: nil } }).where({ managing_organisation: { updated_at: range } }),
+              )
+              .pluck(:id)
+
+      SalesLog.where(id: ids)
     end
 
     def apply_cds_transformation(sales_log, _export_mode)

app/services/exports/user_export_service.rb

@@ -25,14 +25,19 @@ module Exports
       "users_2024_2025_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, _year)
+    def retrieve_resources_from_range(range, _year)
-      if !full_update && recent_export
+      relation = User.left_joins(organisation: :organisation_name_changes)
-        params = { from: recent_export.started_at, to: @start_time }
+      ids = relation
-        User.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params)
+              .where({ updated_at: range })
-      else
+              .or(
-        params = { to: @start_time }
+                relation.where.not(organisations: { updated_at: nil }).where(organisations: { updated_at: range }),
-        User.where("updated_at <= :to", params)
+              )
-      end
+              .or(
+                relation.where(organisation_name_changes: { created_at: range }),
+              )
+              .pluck(:id)
+
+      User.where(id: ids)
     end
 
     def build_export_xml(users)

app/services/exports/xml_export_service.rb

@@ -31,6 +31,16 @@ module Exports
       Export.new(collection:, year:, started_at: @start_time, base_number:, increment_number:)
     end
 
+    def retrieve_resources(recent_export, full_update, year)
+      range = if !full_update && recent_export
+                recent_export.started_at..@start_time
+              else
+                ..@start_time
+              end
+
+      retrieve_resources_from_range(range, year)
+    end
+
     def write_export_archive(export, year, recent_export, full_update)
       archive = get_archive_name(year, export.base_number, export.increment_number)
 

app/services/uprn_client.rb

@@ -39,7 +39,7 @@ private
     client.use_ssl = true
     client.verify_mode = OpenSSL::SSL::VERIFY_PEER
     client.max_retries = 3
-    client.read_timeout = 30 # seconds
+    client.read_timeout = 15 # seconds
     client
   end
 

docs/Gemfile.lock

@@ -226,7 +226,7 @@ GEM
     rb-fsevent (0.11.2)
     rb-inotify (0.10.1)
       ffi (~> 1.0)
-    rexml (3.3.9)
+    rexml (3.4.2)
     rouge (3.26.0)
     ruby2_keywords (0.0.5)
     rubyzip (2.3.2)

spec/services/exports/lettings_log_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::LettingsLogExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -16,7 +18,7 @@ RSpec.describe Exports::LettingsLogExportService do
   let(:expected_manifest_filename) { "manifest.xml" }
   let(:start_time) { Time.zone.local(2022, 5, 1) }
   let(:organisation) { create(:organisation, name: "MHCLG", housing_registration_no: 1234) }
-  let(:user) { FactoryBot.create(:user, email: "test1@example.com", organisation:) }
+  let(:user) { create(:user, email: "test1@example.com", organisation:) }
 
   def replace_entity_ids(lettings_log, export_template)
     export_template.sub!(/\{id\}/, (lettings_log["id"] + Exports::LettingsLogExportService::LOG_ID_OFFSET).to_s)
@@ -480,6 +482,75 @@ RSpec.describe Exports::LettingsLogExportService do
         end
       end
     end
+
+    context "and one lettings log has not been updated in the time range" do
+      let(:expected_zip_filename) { "core_#{current_collection_start_year}_#{current_collection_end_year}_apr_mar_f0001_inc0001.zip" }
+      let(:start_time) { current_collection_start_date }
+      let!(:owning_organisation) { create(:organisation, name: "MHCLG owning", housing_registration_no: 1234) }
+      let!(:managing_organisation) { create(:organisation, name: "MHCLG managing", housing_registration_no: 1234) }
+      let!(:created_by_user) { create(:user, email: "test-created-by@example.com", organisation: managing_organisation) }
+      let!(:updated_by_user) { create(:user, email: "test-updated-by@example.com", organisation: managing_organisation) }
+      let!(:assigned_to_user) { create(:user, email: "test-assigned-to@example.com", organisation: managing_organisation) }
+      let!(:lettings_log) { create(:lettings_log, :completed, startdate: current_collection_start_date, created_by: created_by_user, updated_by: updated_by_user, assigned_to: assigned_to_user, owning_organisation:, managing_organisation:) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        owning_organisation.touch
+        managing_organisation.touch
+        created_by_user.touch
+        updated_by_user.touch
+        assigned_to_user.touch
+        lettings_log.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the lettings log" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if created_by_user is updated" do
+        created_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if updated_by_user is updated" do
+        updated_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if assigned_to_user is updated" do
+        assigned_to_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if owning_organisation is updated" do
+        owning_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if managing_organisation is updated" do
+        managing_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+    end
   end
 
   context "when exporting a supported housing lettings logs in XML" do

spec/services/exports/organisation_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::OrganisationExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -283,5 +285,40 @@ RSpec.describe Exports::OrganisationExportService do
         expect(export_service.export_xml_organisations).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
       end
     end
+
+    context "and one organisation has not been updated in the time range" do
+      let(:dpo_user) { create(:user, email: "dpo@example.com", is_dpo: true, organisation:) }
+      let(:organisation) { create(:organisation, with_dsa: false) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        organisation.touch
+        dpo_user.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the organisation" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_organisations
+      end
+
+      it "does export the organisation if an organisation name change is made" do
+        FactoryBot.create(:organisation_name_change, organisation:)
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_organisations
+      end
+
+      it "does export the organisation if dpo_user is updated" do
+        dpo_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_organisations
+      end
+    end
   end
 end

spec/services/exports/sales_log_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::SalesLogExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -421,5 +423,74 @@ RSpec.describe Exports::SalesLogExportService do
         end
       end
     end
+
+    context "and one sales log has not been updated in the time range" do
+      let(:expected_zip_filename) { "core_sales_#{current_collection_start_year}_#{current_collection_end_year}_apr_mar_f0001_inc0001.zip" }
+      let(:start_time) { current_collection_start_date }
+      let!(:owning_organisation) { create(:organisation, name: "MHCLG owning", housing_registration_no: 1234) }
+      let!(:managing_organisation) { create(:organisation, name: "MHCLG managing", housing_registration_no: 1234) }
+      let!(:created_by_user) { create(:user, email: "test-created-by@example.com", organisation: managing_organisation) }
+      let!(:updated_by_user) { create(:user, email: "test-updated-by@example.com", organisation: managing_organisation) }
+      let!(:assigned_to_user) { create(:user, email: "test-assigned-to@example.com", organisation: managing_organisation) }
+      let!(:sales_log) { create(:sales_log, :export, saledate: start_time, assigned_to: assigned_to_user, created_by: created_by_user, updated_by: updated_by_user, owning_organisation:, managing_organisation:) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        owning_organisation.touch
+        managing_organisation.touch
+        created_by_user.touch
+        updated_by_user.touch
+        assigned_to_user.touch
+        sales_log.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the sales log" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if created_by_user is updated" do
+        created_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if updated_by_user is updated" do
+        updated_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if assigned_to_user is updated" do
+        assigned_to_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if owning_organisation is updated" do
+        owning_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if managing_organisation is updated" do
+        managing_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+    end
   end
 end

spec/services/exports/user_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::UserExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -235,5 +237,40 @@ RSpec.describe Exports::UserExportService do
         expect(export_service.export_xml_users).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
       end
     end
+
+    context "and one user has not been updated in the time range" do
+      let(:start_time) { current_collection_start_date }
+      let!(:user) { create(:user, organisation:) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        organisation.touch
+        user.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the user" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_users
+      end
+
+      it "does export the user if organisation is updated" do
+        organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_users
+      end
+
+      it "does export the user if an organisation name change is made" do
+        FactoryBot.create(:organisation_name_change, organisation:)
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_users
+      end
+    end
   end
 end