Refactor AssetSources to be thread-safe

app/helpers/script_helper.rb

@@ -19,12 +19,12 @@ def render_javascript_pack_once_tags(...)
       return if @scripts.blank?
       concat javascript_assets_tag
       @scripts.each do |name, attributes|
-        AssetSources.get_sources(name).each do |source|
+        asset_sources.get_sources(name).each do |source|
           concat javascript_include_tag(
             source,
             **attributes,
             crossorigin: local_crossorigin_sources? ? true : nil,
-            integrity: AssetSources.get_integrity(source),
+            integrity: asset_sources.get_integrity(source),
           )
         end
       end
@@ -37,12 +37,17 @@ def render_javascript_pack_once_tags(...)
     sprite.svg
   ].to_set.freeze
 
+  def asset_sources
+    Rails.application.config.asset_sources
+  end
+
   def local_crossorigin_sources?
     Rails.env.development? && ENV['WEBPACK_PORT'].present?
   end
 
   def javascript_assets_tag
-    assets = AssetSources.get_assets(*@scripts.keys)
+    assets = asset_sources.get_assets(*@scripts.keys)
+
     if assets.present?
       asset_map = assets.index_with { |path| asset_path(path, host: asset_host(path)) }
       content_tag(

app/services/idv/steps/threat_metrix_step_helper.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Idv
   module Steps
     module ThreatMetrixStepHelper
@@ -19,7 +21,7 @@ def generate_threatmetrix_session_id(updating_ssn)
       # @return [Array<String>]
       def threatmetrix_javascript_urls(session_id)
         sources = if IdentityConfig.store.lexisnexis_threatmetrix_mock_enabled
-                    AssetSources.get_sources('mock-device-profiling')
+                    Rails.application.config.asset_sources.get_sources('mock-device-profiling')
                   else
                     ['https://h.online-metrix.net/fp/tags.js']
                   end

config/application.rb

@@ -38,8 +38,11 @@ class Application < Rails::Application
     )
     IdentityConfig.build_store(configuration)
 
-    AssetSources.manifest_path = Rails.public_path.join('packs', 'manifest.json')
-    AssetSources.cache_manifest = Rails.env.production? || Rails.env.test?
+    config.asset_sources = AssetSources.new(
+      manifest_path: Rails.public_path.join('packs', 'manifest.json'),
+      cache_manifest: Rails.env.production? || Rails.env.test?,
+      i18n_locales: Idp::Constants::AVAILABLE_LOCALES,
+    )
 
     console do
       if ENV['ALLOW_CONSOLE_DB_WRITE_ACCESS'] != 'true' &&

lib/asset_sources.rb

@@ -1,54 +1,67 @@
 # frozen_string_literal: true
 
 class AssetSources
-  class << self
-    attr_accessor :manifest_path
-    attr_accessor :manifest
-    attr_accessor :cache_manifest
-
-    def get_sources(*names)
-      # RailsI18nWebpackPlugin will generate additional assets suffixed per locale, e.g. `.fr.js`.
-      # See: app/javascript/packages/rails-i18n-webpack-plugin/extract-keys-webpack-plugin.js
-      regexp_locale_suffix = %r{\.(#{I18n.available_locales.join('|')})\.js$}
-
-      load_manifest_if_needed
-
-      locale_sources, sources = names.flat_map do |name|
-        manifest&.dig('entrypoints', name, 'assets', 'js')
-      end.uniq.compact.partition { |source| regexp_locale_suffix.match?(source) }
-
-      [
-        *locale_sources.filter { |source| source.end_with? ".#{I18n.locale}.js" },
-        *sources,
-      ]
-    end
+  attr_reader :manifest_path
+  attr_reader :manifest
+  attr_reader :cache_manifest
 
-    def get_assets(*names)
-      load_manifest_if_needed
+  def initialize(manifest_path:, cache_manifest:, i18n_locales:)
+    @manifest_path = manifest_path
+    @cache_manifest = cache_manifest
+    @regexp_locale_suffix = %r{\.(#{i18n_locales.join('|')})\.js$}
 
-      names.flat_map do |name|
-        manifest&.dig('entrypoints', name, 'assets')&.except('js')&.values&.flatten
-      end.uniq.compact
+    if cache_manifest
+      @manifest = read_manifest.freeze
     end
+  end
 
-    def get_integrity(path)
-      load_manifest_if_needed
+  def get_sources(*names)
+    # RailsI18nWebpackPlugin will generate additional assets suffixed per locale, e.g. `.fr.js`.
+    # See: app/javascript/packages/rails-i18n-webpack-plugin/extract-keys-webpack-plugin.js
 
-      manifest&.dig('integrity', path)
-    end
+    load_manifest_if_needed
 
-    def load_manifest
-      self.manifest = begin
-        JSON.parse(File.read(manifest_path))
-      rescue JSON::ParserError, Errno::ENOENT
-        nil
-      end
-    end
+    locale_sources, sources = names.flat_map do |name|
+      manifest&.dig('entrypoints', name, 'assets', 'js')
+    end.uniq.compact.partition { |source| @regexp_locale_suffix.match?(source) }
+
+    [
+      *locale_sources.filter { |source| source.end_with? ".#{I18n.locale}.js" },
+      *sources,
+    ]
+  end
+
+  def get_assets(*names)
+    load_manifest_if_needed
+
+    names.flat_map do |name|
+      manifest&.dig('entrypoints', name, 'assets')&.except('js')&.values&.flatten
+    end.uniq.compact
+  end
 
-    private
+  def get_integrity(path)
+    load_manifest_if_needed
 
-    def load_manifest_if_needed
-      load_manifest if !manifest || !cache_manifest
+    manifest&.dig('integrity', path)
+  end
+
+  def read_manifest
+    return nil if manifest_path.nil?
+
+    begin
+      JSON.parse(File.read(manifest_path))
+    rescue JSON::ParserError, Errno::ENOENT
+      nil
     end
   end
+
+  def load_manifest
+    @manifest = read_manifest
+  end
+
+  private
+
+  def load_manifest_if_needed
+    load_manifest if !manifest || !cache_manifest
+  end
 end

spec/helpers/script_helper_spec.rb

@@ -29,11 +29,14 @@
       before do
         javascript_packs_tag_once('application')
         javascript_packs_tag_once('document-capture', 'document-capture')
-        allow(AssetSources).to receive(:get_sources).with('application').
-          and_return(['/application.js'])
-        allow(AssetSources).to receive(:get_sources).with('document-capture').
-          and_return(['/document-capture.js'])
-        allow(AssetSources).to receive(:get_assets).with('application', 'document-capture').
+        allow(Rails.application.config.asset_sources).to receive(:get_sources).
+          with('application').and_return(['/application.js'])
+        allow(Rails.application.config.asset_sources).to receive(:get_sources).
+          with('document-capture').and_return(['/document-capture.js'])
+        allow(Rails.application.config.asset_sources).to receive(:get_assets).with(
+          'application',
+          'document-capture',
+        ).
           and_return(['clock.svg', 'sprite.svg'])
       end
 
@@ -86,8 +89,9 @@
 
       context 'with script integrity available' do
         before do
-          allow(AssetSources).to receive(:get_integrity).and_return(nil)
-          allow(AssetSources).to receive(:get_integrity).with('/application.js').
+          allow(Rails.application.config.asset_sources).to receive(:get_integrity).and_return(nil)
+          allow(Rails.application.config.asset_sources).to receive(:get_integrity).
+            with('/application.js').
             and_return('sha256-aztp/wpATyjXXpigZtP8ZP/9mUCHDMaL7OKFRbmnUIazQ9ehNmg4CD5Ljzym/TyA')
         end
 
@@ -105,9 +109,9 @@
       context 'with attributes' do
         before do
           javascript_packs_tag_once('track-errors', async: true)
-          allow(AssetSources).to receive(:get_sources).with('track-errors').
-            and_return(['/track-errors.js'])
-          allow(AssetSources).to receive(:get_assets).
+          allow(Rails.application.config.asset_sources).to receive(:get_sources).
+            with('track-errors').and_return(['/track-errors.js'])
+          allow(Rails.application.config.asset_sources).to receive(:get_assets).
             with('application', 'document-capture', 'track-errors').
             and_return([])
         end
@@ -156,7 +160,7 @@
 
     context 'with named scripts argument' do
       before do
-        allow(AssetSources).to receive(:get_sources).with('application').
+        allow(Rails.application.config.asset_sources).to receive(:get_sources).with('application').
           and_return(['/application.js'])
       end
 

spec/lib/asset_sources_spec.rb

@@ -50,21 +50,25 @@
   end
 
   before do
-    AssetSources.manifest = nil
     File.open(manifest_file.path, 'w') { |f| f.puts manifest_content }
-    allow(AssetSources).to receive(:manifest_path).and_return(manifest_file.path)
-    allow(I18n).to receive(:available_locales).and_return([:en, :es, :fr])
     allow(I18n).to receive(:locale).and_return(:en)
   end
 
   after do
     manifest_file.unlink
-    AssetSources.manifest = nil
   end
 
-  describe '.get_sources' do
+  subject(:asset_sources) do
+    AssetSources.new(
+      manifest_path: manifest_file.path, cache_manifest: cache_manifest,
+      i18n_locales: [:en, :es, :fr]
+    )
+  end
+  let(:cache_manifest) { true }
+
+  describe '#get_sources' do
     it 'returns unique localized assets for existing sources, in order, localized scripts first' do
-      expect(AssetSources.get_sources('application', 'application', 'missing', 'input')).to eq [
+      expect(asset_sources.get_sources('application', 'application', 'missing', 'input')).to eq [
         'application.en.js',
         'input.en.js',
         'vendor.js',
@@ -77,34 +81,32 @@
       let(:manifest_content) { nil }
 
       it 'returns an empty array' do
-        expect(AssetSources.get_sources('missing')).to eq([])
+        expect(asset_sources.get_sources('missing')).to eq([])
       end
     end
 
     it 'loads the manifest once' do
-      expect(AssetSources).to receive(:load_manifest).once.and_call_original
+      expect(asset_sources).to_not receive(:load_manifest)
 
-      AssetSources.get_sources('application')
-      AssetSources.get_sources('input')
+      asset_sources.get_sources('application')
+      asset_sources.get_sources('input')
     end
 
     context 'uncached manifest' do
-      before do
-        allow(AssetSources).to receive(:cache_manifest).and_return(false)
-      end
+      let(:cache_manifest) { false }
 
       it 'loads the manifest' do
-        expect(AssetSources).to receive(:load_manifest).twice.and_call_original
+        expect(asset_sources).to receive(:load_manifest).twice.and_call_original
 
-        AssetSources.get_sources('application')
-        AssetSources.get_sources('input')
+        asset_sources.get_sources('application')
+        asset_sources.get_sources('input')
       end
     end
   end
 
-  describe '.get_assets' do
+  describe '#get_assets' do
     it 'returns unique, flattened assets' do
-      expect(AssetSources.get_assets('application', 'application', 'input')).to eq [
+      expect(asset_sources.get_assets('application', 'application', 'input')).to eq [
         'clock.svg',
         'spinner.gif',
       ]
@@ -114,34 +116,32 @@
       let(:manifest_content) { nil }
 
       it 'returns an empty array' do
-        expect(AssetSources.get_assets('missing')).to eq([])
+        expect(asset_sources.get_assets('missing')).to eq([])
       end
     end
 
     it 'loads the manifest once' do
-      expect(AssetSources).to receive(:load_manifest).once.and_call_original
+      expect(asset_sources).to_not receive(:load_manifest)
 
-      AssetSources.get_assets('application')
-      AssetSources.get_assets('input')
+      asset_sources.get_assets('application')
+      asset_sources.get_assets('input')
     end
 
     context 'uncached manifest' do
-      before do
-        allow(AssetSources).to receive(:cache_manifest).and_return(false)
-      end
+      let(:cache_manifest) { false }
 
       it 'loads the manifest' do
-        expect(AssetSources).to receive(:load_manifest).twice.and_call_original
+        expect(asset_sources).to receive(:load_manifest).twice.and_call_original
 
-        AssetSources.get_assets('application')
-        AssetSources.get_assets('input')
+        asset_sources.get_assets('application')
+        asset_sources.get_assets('input')
       end
     end
   end
 
-  describe '.get_integrity' do
+  describe '#get_integrity' do
     let(:path) { 'vendor.js' }
-    subject(:integrity) { AssetSources.get_integrity(path) }
+    subject(:integrity) { asset_sources.get_integrity(path) }
 
     it 'returns the integrity hash' do
       expect(integrity).to start_with('sha256-')
@@ -156,11 +156,11 @@
     end
   end
 
-  describe '.load_manifest' do
+  describe '#load_manifest' do
     it 'sets the manifest' do
-      AssetSources.load_manifest
+      asset_sources.load_manifest
 
-      expect(AssetSources.manifest).to be_kind_of(Hash).and eq(JSON.parse(manifest_content))
+      expect(asset_sources.manifest).to be_kind_of(Hash).and eq(JSON.parse(manifest_content))
     end
 
     context 'missing file' do
@@ -171,19 +171,19 @@
       end
 
       it 'gracefully sets nil manifest' do
-        AssetSources.load_manifest
+        asset_sources.load_manifest
 
-        expect(AssetSources.manifest).to be_nil
+        expect(asset_sources.manifest).to be_nil
       end
     end
 
     context 'invalid json' do
       let(:manifest_content) { '{' }
 
       it 'gracefully sets nil manifest' do
-        AssetSources.load_manifest
+        asset_sources.load_manifest
 
-        expect(AssetSources.manifest).to be_nil
+        expect(asset_sources.manifest).to be_nil
       end
     end
   end