Render the fallback icon for icons that don't exist

Gemfile

@@ -41,7 +41,7 @@ gem 'kaminari'
 gem 'oj', '~> 3.14.0'
 
 # Setup dependencies
-gem 'exercism-config', '>= 0.136.0'
+gem 'exercism-config', '>= 0.137.0'
 # gem 'exercism-config', path: '../config'
 
 # Model-level dependencies

Gemfile.lock

@@ -195,7 +195,7 @@ GEM
     et-orbi (1.2.11)
       tzinfo
     event_stream_parser (1.0.0)
-    exercism-config (0.136.0)
+    exercism-config (0.137.0)
       aws-sdk-dynamodb (~> 1.0)
       aws-sdk-secretsmanager (~> 1.0)
       mandate
@@ -684,7 +684,7 @@ DEPENDENCIES
   devise (~> 4.7)
   discourse_api
   doorkeeper (~> 5.8)
-  exercism-config (>= 0.136.0)
+  exercism-config (>= 0.137.0)
   factory_bot_rails
   friendly_id (~> 5.4.0)
   geocoder (~> 1.8)

app/commands/icons/check_exists.rb

@@ -0,0 +1,18 @@
+# Checks whether an icon actually exists in the icons bucket.
+class Icons::CheckExists
+  include Mandate
+
+  initialize_with :path
+
+  def call
+    # An empty manifest means we couldn't retrieve it, so assume the icon exists
+    # and leave the browser's onerror handler to fall back if it doesn't.
+    return true if manifest.empty?
+
+    manifest.include?(path)
+  end
+
+  private
+  memoize
+  def manifest = Icons::RetrieveManifest.()
+end

app/commands/icons/determine_url_for.rb

@@ -0,0 +1,27 @@
+# Builds the URL for an icon hosted in the icons bucket.
+#
+# Lots of exercises and tracks have never had an icon drawn for them. Without a
+# check here we'd emit a URL for a file that doesn't exist, the browser would
+# request it and get a 403, and only then would our onerror handlers swap in the
+# fallback. S3 errors aren't cacheable at the CDN, so every page view hits the
+# origin again: those missing icons were generating millions of requests a week.
+#
+# So we point straight at the local fallback asset when there isn't an icon.
+class Icons::DetermineUrlFor
+  include Mandate
+  include Propshaft::Helper
+
+  MISSING_EXERCISE_ICON = "graphics/missing-exercise.svg".freeze
+  MISSING_TRACK_ICON = "graphics/missing-track.svg".freeze
+
+  initialize_with :path, :fallback
+
+  def call
+    return fallback_url unless Icons::CheckExists.(path)
+
+    "#{Exercism.config.website_icons_host}/#{path}"
+  end
+
+  private
+  def fallback_url = "#{Rails.application.config.action_controller.asset_host}#{compute_asset_path(fallback)}"
+end

app/commands/icons/retrieve_manifest.rb

@@ -0,0 +1,48 @@
+# Retrieves the set of icon paths that actually exist in the icons bucket.
+#
+# The manifest is published to the bucket by the sync workflow in the
+# exercism/website-icons repo, so it updates whenever icons are added or
+# removed, without needing anything deployed here.
+#
+# This fails open: if we can't retrieve the manifest we return an empty set,
+# which callers treat as "assume everything exists". That keeps us on the old
+# behaviour (request the icon, let the browser's onerror handler fall back)
+# rather than rendering the fallback for every icon on the site.
+class Icons::RetrieveManifest
+  include Mandate
+
+  def call
+    cached = Rails.cache.read(CACHE_KEY)
+    return cached if cached
+
+    # Don't hold onto a failure for the full expiry. A blip would otherwise
+    # leave us checking nothing for an hour.
+    Rails.cache.write(CACHE_KEY, paths, expires_in: paths.empty? ? FAILURE_CACHE_EXPIRY : CACHE_EXPIRY)
+    paths
+  end
+
+  private
+  memoize
+  def paths
+    parsed = JSON.parse(manifest)
+    raise TypeError, "Icons manifest is not an array" unless parsed.is_a?(Array)
+
+    parsed.to_set
+  rescue StandardError => e
+    Sentry.capture_exception(e)
+    Set.new
+  end
+
+  def manifest
+    Exercism.s3_client.get_object(
+      bucket: Exercism.config.aws_icons_bucket,
+      key: MANIFEST_KEY
+    ).body.read
+  end
+
+  CACHE_KEY = "Icons::RetrieveManifest".freeze
+  MANIFEST_KEY = "manifest.json".freeze
+  CACHE_EXPIRY = 1.hour.freeze
+  FAILURE_CACHE_EXPIRY = 1.minute.freeze
+  private_constant :CACHE_KEY, :MANIFEST_KEY, :CACHE_EXPIRY, :FAILURE_CACHE_EXPIRY
+end

app/models/exercise.rb

@@ -167,7 +167,7 @@ def difficulty_category
     end
   end
 
-  def icon_url = "#{Exercism.config.website_icons_host}/exercises/#{icon_name}.svg"
+  def icon_url = Icons::DetermineUrlFor.("exercises/#{icon_name}.svg", Icons::DetermineUrlFor::MISSING_EXERCISE_ICON)
 
   memoize
   def mentoring_notes

app/models/generic_exercise.rb

@@ -11,7 +11,7 @@ def status = super.to_sym
   def git = Git::ProblemSpecifications::Exercise.new(slug)
 
   def url = "https://github.com/exercism/problem-specifications/tree/main/exercises/#{slug}"
-  def icon_url = "#{Exercism.config.website_icons_host}/exercises/#{slug}.svg"
+  def icon_url = Icons::DetermineUrlFor.("exercises/#{slug}.svg", Icons::DetermineUrlFor::MISSING_EXERCISE_ICON)
 
   def self.for!(slug) = find_by!(slug:)
 

app/models/track.rb

@@ -118,7 +118,7 @@ def trophies
     Track::Trophy.for_track(self)
   end
 
-  def icon_url = "#{Exercism.config.website_icons_host}/tracks/#{slug}.svg"
+  def icon_url = Icons::DetermineUrlFor.("tracks/#{slug}.svg", Icons::DetermineUrlFor::MISSING_TRACK_ICON)
 
   def highlightjs_language
     super || slug

test/commands/icons/check_exists_test.rb

@@ -0,0 +1,21 @@
+require 'test_helper'
+
+class Icons::CheckExistsTest < ActiveSupport::TestCase
+  test "true when the icon is in the manifest" do
+    setup_s3_icons_manifest!(["exercises/bob.svg"])
+
+    assert Icons::CheckExists.("exercises/bob.svg")
+  end
+
+  test "false when the icon isn't in the manifest" do
+    setup_s3_icons_manifest!(["exercises/bob.svg"])
+
+    refute Icons::CheckExists.("exercises/flower-field.svg")
+  end
+
+  test "true for everything when the manifest is empty" do
+    setup_s3_icons_manifest!([])
+
+    assert Icons::CheckExists.("exercises/flower-field.svg")
+  end
+end

test/commands/icons/determine_url_for_test.rb

@@ -0,0 +1,33 @@
+require 'test_helper'
+
+class Icons::DetermineUrlForTest < ActiveSupport::TestCase
+  test "returns the bucket url when the icon exists" do
+    setup_s3_icons_manifest!(["exercises/bob.svg"])
+
+    assert_equal "https://assets.exercism.org/exercises/bob.svg",
+      Icons::DetermineUrlFor.("exercises/bob.svg", Icons::DetermineUrlFor::MISSING_EXERCISE_ICON)
+  end
+
+  test "returns the fallback when the icon doesn't exist" do
+    setup_s3_icons_manifest!(["exercises/bob.svg"])
+
+    url = Icons::DetermineUrlFor.("exercises/flower-field.svg", Icons::DetermineUrlFor::MISSING_EXERCISE_ICON)
+    assert_includes url, "missing-exercise"
+    refute_includes url, "flower-field"
+  end
+
+  test "assumes the icon exists when the manifest is unavailable" do
+    assert_equal "https://assets.exercism.org/exercises/flower-field.svg",
+      Icons::DetermineUrlFor.("exercises/flower-field.svg", Icons::DetermineUrlFor::MISSING_EXERCISE_ICON)
+  end
+
+  test "exercise and track icon urls use the manifest" do
+    setup_s3_icons_manifest!(["tracks/ruby.svg"])
+
+    track = create :track, slug: :ruby
+    exercise = create :practice_exercise, track:, slug: :flower_field, icon_name: 'flower-field'
+
+    assert_equal "https://assets.exercism.org/tracks/ruby.svg", track.icon_url
+    assert_includes exercise.icon_url, "missing-exercise"
+  end
+end

test/commands/icons/retrieve_manifest_test.rb

@@ -0,0 +1,32 @@
+require 'test_helper'
+
+class Icons::RetrieveManifestTest < ActiveSupport::TestCase
+  test "retrieves and caches the manifest" do
+    setup_s3_icons_manifest!(["exercises/bob.svg", "tracks/ruby.svg"])
+
+    assert_equal Set["exercises/bob.svg", "tracks/ruby.svg"], Icons::RetrieveManifest.()
+
+    # The second call should come from the cache, not the bucket
+    setup_s3_icons_manifest!(["exercises/leap.svg"])
+    assert_equal Set["exercises/bob.svg", "tracks/ruby.svg"], Icons::RetrieveManifest.()
+  end
+
+  test "returns an empty set when there's no manifest" do
+    assert_empty Icons::RetrieveManifest.()
+  end
+
+  test "returns an empty set when the manifest isn't an array" do
+    setup_s3_icons_manifest!({ exercises: [] })
+
+    assert_empty Icons::RetrieveManifest.()
+  end
+
+  test "retries sooner after a failure" do
+    assert_empty Icons::RetrieveManifest.()
+
+    travel 2.minutes do
+      setup_s3_icons_manifest!(["exercises/bob.svg"])
+      assert_equal Set["exercises/bob.svg"], Icons::RetrieveManifest.()
+    end
+  end
+end

test/test_helper.rb

@@ -187,6 +187,7 @@ class ActiveSupport::TestCase
 
   teardown do
     reset_opensearch!
+    remove_s3_icons_manifest!
 
     Bullet.perform_out_of_channel_notifications if Bullet.notification?
     Bullet.end_request
@@ -295,6 +296,33 @@ def create_tooling_job!(submission, type, params = {})
     )
   end
 
+  # The icons bucket and its manifest live in production. Tests that care about
+  # icon urls create the bucket in LocalStack and upload their own manifest.
+  # Everything else gets a NoSuchBucket, which the command treats as "assume
+  # every icon exists" and so leaves urls untouched.
+  def setup_s3_icons_manifest!(paths)
+    @__uploaded_s3_icons_manifest__ = true
+
+    begin
+      Exercism.s3_client.create_bucket(bucket: Exercism.config.aws_icons_bucket)
+    rescue Aws::S3::Errors::BucketAlreadyOwnedByYou, Aws::S3::Errors::BucketAlreadyExists
+      # Already there from a previous test
+    end
+
+    upload_to_s3(Exercism.config.aws_icons_bucket, ICONS_MANIFEST_KEY, paths.to_json)
+  end
+
+  # S3 isn't rolled back between tests the way the database is, so a manifest
+  # left behind would change icon urls in every test that ran after it.
+  def remove_s3_icons_manifest!
+    return unless @__uploaded_s3_icons_manifest__
+
+    Exercism.s3_client.delete_object(bucket: Exercism.config.aws_icons_bucket, key: ICONS_MANIFEST_KEY)
+    @__uploaded_s3_icons_manifest__ = false
+  end
+
+  ICONS_MANIFEST_KEY = "manifest.json".freeze
+
   # The cache bucket is created by terraform in production. Locally the
   # config key doesn't exist, so tests set it and create the bucket in
   # LocalStack on demand.