DEV: Refactor svg sprite parsing (#20727)

There was a lot of duplication in the svg parsing and coercion code. This reduces that duplication and causes svg sprite parsing to happen earlier so that more computation is cached.
This commit is contained in:
Daniel Waterworth 2023-03-20 11:41:23 -05:00 committed by GitHub
parent fa96569ef2
commit da0d20d4a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 58 additions and 122 deletions

View File

@ -243,22 +243,37 @@ module SvgSprite
badge_icons
end
def self.core_svg_sprites
@core_svg_sprites ||=
begin
CORE_SVG_SPRITES.map do |path|
{ filename: File.basename(path, ".svg"), sprite: File.read(path) }
def self.symbols_for(svg_filename, sprite, strict:)
if strict
Nokogiri.XML(sprite) { |config| config.options = Nokogiri::XML::ParseOptions::NOBLANKS }
else
Nokogiri.XML(sprite)
end.css("symbol")
.filter_map do |sym|
icon_id = prepare_symbol(sym, svg_filename)
if icon_id.present?
sym.attributes["id"].value = icon_id
sym.css("title").each(&:remove)
[icon_id, sym.to_xml]
end
end
.to_h
end
def self.core_svgs
@core_svgs ||=
CORE_SVG_SPRITES.reduce({}) do |symbols, path|
symbols.merge!(symbols_for(File.basename(path, ".svg"), File.read(path), strict: true))
end
end
# Just used in tests
def self.clear_plugin_svg_sprite_cache!
@plugin_svg_sprites = nil
@plugin_svgs = nil
end
def self.plugin_svg_sprites
@plugin_svg_sprites ||=
def self.plugin_svgs
@plugin_svgs ||=
begin
plugin_paths = []
Discourse
@ -268,13 +283,13 @@ module SvgSprite
custom_sprite_paths = Dir.glob(plugin_paths)
custom_sprite_paths.map do |path|
{ filename: File.basename(path, ".svg"), sprite: File.read(path) }
custom_sprite_paths.reduce({}) do |symbols, path|
symbols.merge!(symbols_for(File.basename(path, ".svg"), File.read(path), strict: true))
end
end
end
def self.theme_svg_sprites(theme_id)
def self.theme_svgs(theme_id)
if theme_id.present?
theme_ids = Theme.transform_ids(theme_id)
@ -295,17 +310,25 @@ module SvgSprite
)
end
theme_sprites.map do |upload_id, sprite|
{ filename: "theme_#{theme_id}_#{upload_id}.svg", sprite: sprite }
theme_sprites.reduce({}) do |symbols, (upload_id, sprite)|
begin
symbols.merge!(symbols_for("theme_#{theme_id}_#{upload_id}.svg", sprite, strict: false))
rescue => e
Rails.logger.warn(
"Bad XML in custom sprite in theme with ID=#{theme_id}. Error info: #{e.inspect}",
)
end
symbols
end
end
else
[]
{}
end
end
def self.custom_svg_sprites(theme_id)
plugin_svg_sprites + theme_svg_sprites(theme_id)
def self.custom_svgs(theme_id)
plugin_svgs.merge(theme_svgs(theme_id))
end
def self.all_icons(theme_id = nil)
@ -339,34 +362,10 @@ module SvgSprite
cache&.clear
end
def self.sprites_for(theme_id)
sprites = core_svg_sprites
sprites += custom_svg_sprites(theme_id) if theme_id.present?
sprites
end
def self.core_svgs
@core_svgs ||=
begin
symbols = {}
CORE_SVG_SPRITES.each do |filename|
svg_filename = "#{File.basename(filename, ".svg")}"
Nokogiri
.XML(File.open(filename)) do |config|
config.options = Nokogiri::XML::ParseOptions::NOBLANKS
end
.css("symbol")
.each do |sym|
icon_id = prepare_symbol(sym, svg_filename)
sym.attributes["id"].value = icon_id
symbols[icon_id] = sym.to_xml
end
end
symbols
end
def self.svgs_for(theme_id)
svgs = core_svgs
svgs = svgs.merge(custom_svgs(theme_id)) if theme_id.present?
svgs
end
def self.bundle(theme_id = nil)
@ -382,34 +381,8 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
" \
"".dup
core_svgs.each { |icon_id, sym| svg_subset << sym if icons.include?(icon_id) }
custom_svg_sprites(theme_id).each do |item|
begin
svg_file =
Nokogiri.XML(item[:sprite]) do |config|
config.options = Nokogiri::XML::ParseOptions::NOBLANKS
end
rescue => e
Rails.logger.warn(
"Bad XML in custom sprite in theme with ID=#{theme_id}. Error info: #{e.inspect}",
)
end
next if !svg_file
svg_file
.css("symbol")
.each do |sym|
icon_id = prepare_symbol(sym, item[:filename])
if icon_id.present?
sym.attributes["id"].value = icon_id
sym.css("title").each(&:remove)
svg_subset << sym.to_xml
end
end
end
svg_subset << core_svgs.slice(*icons).values.join
svg_subset << custom_svgs(theme_id).values.join
svg_subset << "</svg>"
end
@ -417,46 +390,16 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
def self.search(searched_icon)
searched_icon = process(searched_icon.dup)
sprites_for(SiteSetting.default_theme_id).each do |item|
svg_file = Nokogiri.XML(item[:sprite])
svg_file
.css("symbol")
.each do |sym|
icon_id = prepare_symbol(sym, item[:filename])
if searched_icon == icon_id
sym.attributes["id"].value = icon_id
sym.css("title").each(&:remove)
return sym.to_xml
end
end
end
false
svgs_for(SiteSetting.default_theme_id)[searched_icon] || false
end
def self.icon_picker_search(keyword, only_available = false)
icons = all_icons(SiteSetting.default_theme_id) if only_available
results = Set.new
sprites_for(SiteSetting.default_theme_id).each do |item|
svg_file = Nokogiri.XML(item[:sprite])
svg_file
.css("symbol")
.each do |sym|
icon_id = prepare_symbol(sym, item[:filename])
next if only_available && !icons.include?(icon_id)
if keyword.empty? || icon_id.include?(keyword)
sym.attributes["id"].value = icon_id
sym.css("title").each(&:remove)
results.add(id: icon_id, symbol: sym.to_xml)
end
end
end
results.sort_by { |icon| icon[:id] }
symbols = svgs_for(SiteSetting.default_theme_id)
symbols.slice!(*icons) if only_available
symbols.reject! { |icon_id, sym| !icon_id.include?(keyword) } unless keyword.empty?
symbols.sort_by(&:first).map { |icon_id, symbol| { id: icon_id, symbol: symbol } }
end
# For use in no_ember .html.erb layouts
@ -539,14 +482,7 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
def self.custom_icons(theme_id)
# Automatically register icons in sprites added via themes or plugins
icons = []
custom_svg_sprites(theme_id).each do |item|
svg_file = Nokogiri.XML(item[:sprite])
svg_file
.css("symbol")
.each { |sym| icons << sym.attributes["id"].value if sym.attributes["id"].present? }
end
icons
custom_svgs(theme_id).keys
end
def self.process(icon_name)

View File

@ -184,7 +184,7 @@ RSpec.describe SvgSprite do
)
theme.save!
sprite_files = SvgSprite.custom_svg_sprites(theme.id).join("|")
sprite_files = SvgSprite.custom_svgs(theme.id).values.join("|")
expect(sprite_files).to match(/my-custom-theme-icon/)
SvgSprite.bundle(theme.id)
@ -230,9 +230,9 @@ RSpec.describe SvgSprite do
expect(SvgSprite.bundle).to match(/far-building/)
end
describe "#custom_svg_sprites" do
describe "#custom_svgs" do
it "is empty by default" do
expect(SvgSprite.custom_svg_sprites(nil)).to be_empty
expect(SvgSprite.custom_svgs(nil)).to be_empty
expect(SvgSprite.bundle).not_to be_empty
end
@ -254,7 +254,7 @@ RSpec.describe SvgSprite do
end
it "includes custom icons from plugins" do
expect(SvgSprite.custom_svg_sprites(nil).size).to eq(1)
expect(SvgSprite.custom_svgs(nil).size).to eq(1)
expect(SvgSprite.bundle).to match(/custom-icon/)
end
end

View File

@ -612,7 +612,7 @@ HTML
describe "SVG sprite theme fields" do
let :svg_content do
"<svg></svg>"
"<svg><symbol id='test'></symbol></svg>"
end
let :upload_file do
@ -651,10 +651,10 @@ HTML
it "clears SVG sprite cache when upload is deleted" do
theme_field
expect(SvgSprite.custom_svg_sprites(theme.id).size).to eq(1)
expect(SvgSprite.custom_svgs(theme.id).size).to eq(1)
theme_field.destroy!
expect(SvgSprite.custom_svg_sprites(theme.id).size).to eq(0)
expect(SvgSprite.custom_svgs(theme.id).size).to eq(0)
end
it "crashes gracefully when svg is invalid" do