From 59eb6401db08f540a092e5e404065cb402267358 Mon Sep 17 00:00:00 2001 From: MAEDA Go Date: Wed, 2 Sep 2026 11:54:54 +0900 Subject: [PATCH 1/2] Memoize theme icon names instead of fetching them from the cache store for every icon --- lib/redmine/themes.rb | 13 ++++++++++--- test/unit/lib/redmine/themes_test.rb | 16 +++------------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/lib/redmine/themes.rb b/lib/redmine/themes.rb index 601b8abd3..0739d0eb6 100644 --- a/lib/redmine/themes.rb +++ b/lib/redmine/themes.rb @@ -116,9 +116,16 @@ module Redmine return [] unless asset - ActionController::Base.cache_store.fetch("theme-icons/#{id}/#{sprite}/#{asset.digest}") do - asset.content.scan(/id=['"]icon--([^'"]+)['"]/).flatten - end + # This is called once per icon rendered, so it must not read and + # scan the sprite every time. Memoize the names per asset digest, + # so that changes to the sprite file are still picked up. + @icons ||= {} + digest, names = @icons[sprite] + return names if digest == asset.digest + + names = asset.content.scan(/id=['"]icon--([^'"]+)['"]/).flatten + @icons[sprite] = [asset.digest, names] + names end def asset_paths diff --git a/test/unit/lib/redmine/themes_test.rb b/test/unit/lib/redmine/themes_test.rb index f24df6ea4..5e09e7d69 100644 --- a/test/unit/lib/redmine/themes_test.rb +++ b/test/unit/lib/redmine/themes_test.rb @@ -82,29 +82,19 @@ class Redmine::ThemesTest < ActiveSupport::TestCase assert_equal [], theme.icons('icons') end - def test_icons_should_be_cached + def test_icons_should_pick_up_changes_to_the_sprite theme = Redmine::Themes::Theme.new('/tmp/test') - theme.stubs(:id).returns('test') theme.stubs(:image_path).with('icons.svg').returns('themes/test/icons.svg') asset = mock('asset') - asset.stubs(:content).returns('') asset.stubs(:digest).returns('123456') + asset.stubs(:content).returns('') Rails.application.assets.load_path.stubs(:find).with('themes/test/icons.svg').returns(asset) - # Use a memory store for this test since the test environment uses null_store - memory_store = ActiveSupport::Cache.lookup_store(:memory_store) - ActionController::Base.stubs(:cache_store).returns(memory_store) - - # First call - cache miss assert_equal ['edit'], theme.icons('icons') - # Second call - verify it's in the cache - cache_key = "theme-icons/test/icons/123456" - assert_equal ['edit'], memory_store.read(cache_key) - - # If digest changes, it should miss cache + # A new digest must invalidate the memoized names asset.stubs(:digest).returns('789') asset.stubs(:content).returns('') assert_equal ['new'], theme.icons('icons') -- 2.55.0