From d0243f741eaddf6004b7b740b9a2fc948e33cb08 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 20 Sep 2022 10:28:17 +0100 Subject: [PATCH] UX: Use dominant color as image loading placeholder (#18248) We previously had a system which would generate a 10x10px preview of images and add their URLs in a data-small-upload attribute. The client would then use that as the background-image of the `` element. This works reasonably well on fast connections, but on slower connections it can take a few seconds for the placeholders to appear. The act of loading the placeholders can also break or delay the loading of the 'real' images. This commit replaces the placeholder logic with a new approach. Instead of a 10x10px preview, we use imagemagick to calculate the average color of an image and store it in the database. The hex color value then added as a `data-dominant-color` attribute on the `` element, and the client can use this as a `background-color` on the element while the real image is loading. That means no extra HTTP request is required, and so the placeholder color can appear instantly. Dominant color will be calculated: 1. When a new upload is created 2. During a post rebake, if the dominant color is missing from an upload, it will be calculated and stored 3. Every 15 minutes, 25 old upload records are fetched and their dominant color calculated and stored. (part of the existing PeriodicalUpdates job) Existing posts will continue to use the old 10x10px placeholder system until they are next rebaked --- .../discourse/app/lib/lazy-load-images.js | 51 ++++++------- app/jobs/scheduled/periodical_updates.rb | 2 + app/models/upload.rb | 76 ++++++++++++++++++- app/serializers/upload_serializer.rb | 3 +- ...915132547_add_dominant_color_to_uploads.rb | 8 ++ lib/cooked_post_processor.rb | 20 ++--- lib/discourse.rb | 17 ++++- lib/upload_creator.rb | 1 + spec/fabricators/upload_fabricator.rb | 6 +- spec/lib/cooked_post_processor_spec.rb | 51 +++++-------- spec/models/upload_spec.rb | 72 ++++++++++++++++++ .../schemas/json/upload_create_response.json | 3 + 12 files changed, 232 insertions(+), 78 deletions(-) create mode 100644 db/migrate/20220915132547_add_dominant_color_to_uploads.rb diff --git a/app/assets/javascripts/discourse/app/lib/lazy-load-images.js b/app/assets/javascripts/discourse/app/lib/lazy-load-images.js index 5b79ebe17dd..d905ec313f3 100644 --- a/app/assets/javascripts/discourse/app/lib/lazy-load-images.js +++ b/app/assets/javascripts/discourse/app/lib/lazy-load-images.js @@ -1,14 +1,3 @@ -// Min size in pixels for consideration for lazy loading -const MINIMUM_SIZE = 150; - -function forEachImage(post, callback) { - post.querySelectorAll("img").forEach((img) => { - if (img.width >= MINIMUM_SIZE && img.height >= MINIMUM_SIZE) { - callback(img); - } - }); -} - function isLoaded(img) { // In Safari, img.complete sometimes returns true even when the image is not loaded. // naturalHeight seems to be a more reliable check @@ -16,40 +5,48 @@ function isLoaded(img) { } export function nativeLazyLoading(api) { + api.decorateCookedElement( + (post) => + post.querySelectorAll("img").forEach((img) => (img.loading = "lazy")), + { + id: "discourse-lazy-load", + } + ); + api.decorateCookedElement( (post) => { const siteSettings = api.container.lookup("service:site-settings"); - forEachImage(post, (img) => { - img.loading = "lazy"; + post.querySelectorAll("img").forEach((img) => { + // Support for smallUpload should be maintained until Post::BAKED_VERSION is bumped higher than 2 + const { smallUpload, dominantColor } = img.dataset; - if (siteSettings.secure_media) { + if (siteSettings.secure_media && smallUpload) { // Secure media requests go through the app. In topics with many images, // this makes it very easy to hit rate limiters. Skipping the low-res // placeholders reduces the chance of this problem occuring. return; } - if (img.dataset.smallUpload) { - if (!isLoaded(img)) { - if (!img.onload) { - img.onload = () => { - img.style.removeProperty("background-image"); - img.style.removeProperty("background-size"); - }; - } + if ((smallUpload || dominantColor) && !isLoaded(img)) { + if (!img.onload) { + img.onload = () => { + img.style.removeProperty("background-image"); + img.style.removeProperty("background-size"); + img.style.removeProperty("background-color"); + }; + } - img.style.setProperty( - "background-image", - `url(${img.dataset.smallUpload})` - ); + if (smallUpload) { + img.style.setProperty("background-image", `url(${smallUpload})`); img.style.setProperty("background-size", "cover"); + } else { + img.style.setProperty("background-color", `#${dominantColor}`); } } }); }, { - onlyStream: true, id: "discourse-lazy-load-after-adopt", afterAdopt: true, } diff --git a/app/jobs/scheduled/periodical_updates.rb b/app/jobs/scheduled/periodical_updates.rb index 5c5da8bac6b..01649de0627 100644 --- a/app/jobs/scheduled/periodical_updates.rb +++ b/app/jobs/scheduled/periodical_updates.rb @@ -48,6 +48,8 @@ module Jobs Category.auto_bump_topic! + Upload.backfill_dominant_colors!(25) + nil end diff --git a/app/models/upload.rb b/app/models/upload.rb index 11fb98d0e15..08a7213d047 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -10,6 +10,7 @@ class Upload < ActiveRecord::Base SEEDED_ID_THRESHOLD = 0 URL_REGEX ||= /(\/original\/\dX[\/\.\w]*\/(\h+)[\.\w]*)/ MAX_IDENTIFY_SECONDS = 5 + DOMINANT_COLOR_COMMAND_TIMEOUT_SECONDS = 5 belongs_to :user belongs_to :access_control_post, class_name: 'Post' @@ -316,6 +317,71 @@ class Upload < ActiveRecord::Base get_dimension(:thumbnail_height) end + def dominant_color(calculate_if_missing: false) + val = read_attribute(:dominant_color) + if val.nil? && calculate_if_missing + calculate_dominant_color! + read_attribute(:dominant_color) + else + val + end + end + + def calculate_dominant_color!(local_path = nil) + color = nil + + if !FileHelper.is_supported_image?("image.#{extension}") || extension == "svg" + color = "" + end + + if color.nil? + local_path ||= + if local? + Discourse.store.path_for(self) + else + Discourse.store.download(self).path + end + + color = begin + data = Discourse::Utils.execute_command( + "nice", + "-n", + "10", + "convert", + local_path, + "-resize", + "1x1", + "-define", + "histogram:unique-colors=true", + "-format", + "%c", + "histogram:info:", + timeout: DOMINANT_COLOR_COMMAND_TIMEOUT_SECONDS + ) + + # Output format: + # 1: (110.873,116.226,93.8821) #6F745E srgb(43.4798%,45.5789%,36.8165%) + + color = data[/#([0-9A-F]{6})/, 1] + + raise "Calculated dominant color but unable to parse output:\n#{data}" if color.nil? + + color + rescue Discourse::Utils::CommandError => e + # Timeout or unable to parse image + # This can happen due to bad user input - ignore and save + # an empty string to prevent re-evaluation + "" + end + end + + if persisted? + self.update_column(:dominant_color, color) + else + self.dominant_color = color + end + end + def target_image_quality(local_path, test_quality) @file_quality ||= Discourse::Utils.execute_command("identify", "-format", "%Q", local_path, timeout: MAX_IDENTIFY_SECONDS).to_i rescue 0 @@ -522,6 +588,12 @@ class Upload < ActiveRecord::Base Upload.where(sha1: sha1s.uniq).pluck(:id) end + def self.backfill_dominant_colors!(count) + Upload.where(dominant_color: nil).order("id desc").first(count).each do |upload| + upload.calculate_dominant_color! + end + end + private def short_url_basename @@ -553,10 +625,11 @@ end # secure :boolean default(FALSE), not null # access_control_post_id :bigint # original_sha1 :string -# verification_status :integer default(1), not null # animated :boolean +# verification_status :integer default(1), not null # security_last_changed_at :datetime # security_last_changed_reason :string +# dominant_color :text # # Indexes # @@ -564,6 +637,7 @@ end # index_uploads_on_access_control_post_id (access_control_post_id) # index_uploads_on_etag (etag) # index_uploads_on_extension (lower((extension)::text)) +# index_uploads_on_id (id) WHERE (dominant_color IS NULL) # index_uploads_on_id_and_url (id,url) # index_uploads_on_original_sha1 (original_sha1) # index_uploads_on_sha1 (sha1) UNIQUE diff --git a/app/serializers/upload_serializer.rb b/app/serializers/upload_serializer.rb index 43b89526f91..a31a6d02100 100644 --- a/app/serializers/upload_serializer.rb +++ b/app/serializers/upload_serializer.rb @@ -13,7 +13,8 @@ class UploadSerializer < ApplicationSerializer :short_url, :short_path, :retain_hours, - :human_filesize + :human_filesize, + :dominant_color def url object.for_site_setting ? object.url : UrlHelper.cook_url(object.url, secure: SiteSetting.secure_media? && object.secure) diff --git a/db/migrate/20220915132547_add_dominant_color_to_uploads.rb b/db/migrate/20220915132547_add_dominant_color_to_uploads.rb new file mode 100644 index 00000000000..1b60a4b191b --- /dev/null +++ b/db/migrate/20220915132547_add_dominant_color_to_uploads.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddDominantColorToUploads < ActiveRecord::Migration[7.0] + def change + add_column :uploads, :dominant_color, :text, limit: 6, null: true + add_index :uploads, :id, where: "dominant_color IS NULL" + end +end diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 2f27ecdf466..f2a3717ab19 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -7,8 +7,6 @@ class CookedPostProcessor include CookedProcessorMixin LIGHTBOX_WRAPPER_CSS_CLASS = "lightbox-wrapper" - LOADING_SIZE = 10 - LOADING_COLORS = 32 GIF_SOURCES_REGEXP = /(giphy|tenor)\.com\// attr_reader :cooking_options, :doc @@ -32,7 +30,7 @@ class CookedPostProcessor @has_oneboxes = post.post_analyzer.found_oneboxes? @size_cache = {} - @disable_loading_image = !!opts[:disable_loading_image] + @disable_dominant_color = !!opts[:disable_dominant_color] @omit_nofollow = post.omit_nofollow? end @@ -193,10 +191,6 @@ class CookedPostProcessor end end - unless @disable_loading_image - upload.create_thumbnail!(LOADING_SIZE, LOADING_SIZE, format: 'png', colors: LOADING_COLORS) - end - return if upload.animated? if img.ancestors('.onebox, .onebox-body, .quote').blank? && !img.classes.include?("onebox") @@ -207,10 +201,6 @@ class CookedPostProcessor end end - def loading_image(upload) - upload.thumbnail(LOADING_SIZE, LOADING_SIZE) - end - def each_responsive_ratio SiteSetting .responsive_post_image_sizes @@ -223,7 +213,7 @@ class CookedPostProcessor def optimize_image!(img, upload, cropped: false) w, h = img["width"].to_i, img["height"].to_i - # note: optimize_urls cooks the src and data-small-upload further after this + # note: optimize_urls cooks the src further after this thumbnail = upload.thumbnail(w, h) if thumbnail && thumbnail.filesize.to_i < upload.filesize img["src"] = thumbnail.url @@ -248,8 +238,8 @@ class CookedPostProcessor img["src"] = upload.url end - if small_upload = loading_image(upload) - img["data-small-upload"] = small_upload.url + if !@disable_dominant_color && (color = upload.dominant_color(calculate_if_missing: true).presence) + img["data-dominant-color"] = color end end @@ -329,7 +319,7 @@ class CookedPostProcessor end end - %w{src data-small-upload}.each do |selector| + %w{src}.each do |selector| @doc.css("img[#{selector}]").each do |img| custom_emoji = img["class"]&.include?("emoji-custom") && Emoji.custom?(img["title"]) img[selector] = UrlHelper.cook_url( diff --git a/lib/discourse.rb b/lib/discourse.rb index 3abaafaa7b3..9bd00a2e640 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -109,6 +109,16 @@ module Discourse nil end + class CommandError < RuntimeError + attr_reader :status, :stdout, :stderr + def initialize(message, status: nil, stdout: nil, stderr: nil) + super(message) + @status = status + @stdout = stdout + @stderr = stderr + end + end + private class CommandRunner @@ -145,7 +155,12 @@ module Discourse if !status.exited? || !success_status_codes.include?(status.exitstatus) failure_message = "#{failure_message}\n" if !failure_message.blank? - raise "#{caller[0]}: #{failure_message}#{stderr}" + raise CommandError.new( + "#{caller[0]}: #{failure_message}#{stderr}", + stdout: stdout, + stderr: stderr, + status: status + ) end stdout diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb index 5aa02520a20..8b5655c4b3b 100644 --- a/lib/upload_creator.rb +++ b/lib/upload_creator.rb @@ -168,6 +168,7 @@ class UploadCreator @upload.thumbnail_width, @upload.thumbnail_height = ImageSizer.resize(w, h) @upload.width, @upload.height = w, h @upload.animated = animated? + @upload.calculate_dominant_color!(@file.path) end add_metadata! diff --git a/spec/fabricators/upload_fabricator.rb b/spec/fabricators/upload_fabricator.rb index b7b8b90d669..5ccd0b96118 100644 --- a/spec/fabricators/upload_fabricator.rb +++ b/spec/fabricators/upload_fabricator.rb @@ -22,9 +22,11 @@ Fabricator(:upload) do end Fabricator(:image_upload, from: :upload) do - after_create do |upload| + transient color: "white" + + after_create do |upload, transients| file = Tempfile.new(['fabricated', '.png']) - `convert -size #{upload.width}x#{upload.height} xc:white "#{file.path}"` + `convert -size #{upload.width}x#{upload.height} xc:#{transients[:color]} "#{file.path}"` upload.url = Discourse.store.store_upload(file, upload) upload.sha1 = Upload.generate_digest(file.path) diff --git a/spec/lib/cooked_post_processor_spec.rb b/spec/lib/cooked_post_processor_spec.rb index a4515487e08..e808b0116cc 100644 --- a/spec/lib/cooked_post_processor_spec.rb +++ b/spec/lib/cooked_post_processor_spec.rb @@ -14,7 +14,7 @@ RSpec.describe CookedPostProcessor do RAW end - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) } let(:post_process) { sequence("post_process") } it "post process in sequence" do @@ -258,7 +258,7 @@ RSpec.describe CookedPostProcessor do before { SiteSetting.responsive_post_image_sizes = "1|1.5|3" } it "includes responsive images on demand" do - upload.update!(width: 2000, height: 1500, filesize: 10000) + upload.update!(width: 2000, height: 1500, filesize: 10000, dominant_color: "FFFFFF") post = Fabricate(:post, raw: "hello ") # fake some optimized images @@ -284,17 +284,6 @@ RSpec.describe CookedPostProcessor do filesize: 800 ) - # Fake a loading image - _optimized_image = OptimizedImage.create!( - url: "/#{upload_path}/10x10.png", - width: CookedPostProcessor::LOADING_SIZE, - height: CookedPostProcessor::LOADING_SIZE, - upload_id: upload.id, - sha1: SecureRandom.hex, - extension: '.png', - filesize: 123 - ) - cpp = CookedPostProcessor.new(post) cpp.add_to_size_cache(upload.url, 2000, 1500) @@ -302,7 +291,7 @@ RSpec.describe CookedPostProcessor do html = cpp.html - expect(html).to include(%Q|data-small-upload="//test.localhost/#{upload_path}/10x10.png"|) + expect(html).to include(%Q|data-dominant-color="FFFFFF"|) # 1.5x is skipped cause we have a missing thumb expect(html).to include("srcset=\"//test.localhost/#{upload_path}/666x500.jpg, //test.localhost/#{upload_path}/1998x1500.jpg 3x\"") expect(html).to include("src=\"//test.localhost/#{upload_path}/666x500.jpg\"") @@ -316,7 +305,7 @@ RSpec.describe CookedPostProcessor do html = cpp.html - expect(html).to include(%Q|data-small-upload="//cdn.localhost/#{upload_path}/10x10.png"|) + expect(html).to include(%Q|data-dominant-color="FFFFFF"|) expect(html).to include("srcset=\"//cdn.localhost/#{upload_path}/666x500.jpg, //cdn.localhost/#{upload_path}/1998x1500.jpg 3x\"") expect(html).to include("src=\"//cdn.localhost/#{upload_path}/666x500.jpg\"") end @@ -416,7 +405,7 @@ RSpec.describe CookedPostProcessor do HTML end - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) } before do SiteSetting.max_image_height = 2000 @@ -556,7 +545,7 @@ RSpec.describe CookedPostProcessor do HTML end - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) } before do SiteSetting.create_thumbnails = true @@ -580,7 +569,7 @@ RSpec.describe CookedPostProcessor do HTML end - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) } before do SiteSetting.create_thumbnails = true @@ -604,7 +593,7 @@ RSpec.describe CookedPostProcessor do HTML end - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) } before do SiteSetting.create_thumbnails = true @@ -631,7 +620,7 @@ RSpec.describe CookedPostProcessor do HTML end - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) } before do set_subfolder "/subfolder" @@ -671,7 +660,7 @@ RSpec.describe CookedPostProcessor do HTML end - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) } before do SiteSetting.max_image_height = 2000 @@ -699,7 +688,7 @@ RSpec.describe CookedPostProcessor do HTML end - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) } before do SiteSetting.max_image_height = 2000 @@ -727,7 +716,7 @@ RSpec.describe CookedPostProcessor do HTML end - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) } before do SiteSetting.max_image_height = 2000 @@ -817,7 +806,7 @@ RSpec.describe CookedPostProcessor do ![alttext|1750x2000|thumbnail](#{upload2.url}) MD - CookedPostProcessor.new(post, disable_loading_image: true).post_process + CookedPostProcessor.new(post, disable_dominant_color: true).post_process expect(post.reload.image_upload_id).to eq(upload2.id) end @@ -959,7 +948,7 @@ RSpec.describe CookedPostProcessor do it "adds lightbox and optimizes images" do post = Fabricate(:post, raw: "![image|1024x768, 50%](#{upload.short_url})") - cpp = CookedPostProcessor.new(post, disable_loading_image: true) + cpp = CookedPostProcessor.new(post, disable_dominant_color: true) cpp.post_process doc = Nokogiri::HTML5::fragment(cpp.html) @@ -974,7 +963,7 @@ RSpec.describe CookedPostProcessor do upload.update!(animated: true) post = Fabricate(:post, raw: "![image|1024x768, 50%](#{upload.short_url})") - cpp = CookedPostProcessor.new(post, disable_loading_image: true) + cpp = CookedPostProcessor.new(post, disable_dominant_color: true) cpp.post_process doc = Nokogiri::HTML5::fragment(cpp.html) @@ -992,7 +981,7 @@ RSpec.describe CookedPostProcessor do it "marks giphy images as animated" do post = Fabricate(:post, raw: "![tennis-gif|311x280](https://media2.giphy.com/media/7Oifk90VrCdNe/giphy.webp)") - cpp = CookedPostProcessor.new(post, disable_loading_image: true) + cpp = CookedPostProcessor.new(post, disable_dominant_color: true) cpp.post_process doc = Nokogiri::HTML5::fragment(cpp.html) @@ -1001,7 +990,7 @@ RSpec.describe CookedPostProcessor do it "marks giphy images as animated" do post = Fabricate(:post, raw: "![cat](https://media1.tenor.com/images/20c7ddd5e84c7427954f430439c5209d/tenor.gif)") - cpp = CookedPostProcessor.new(post, disable_loading_image: true) + cpp = CookedPostProcessor.new(post, disable_dominant_color: true) cpp.post_process doc = Nokogiri::HTML5::fragment(cpp.html) @@ -1016,7 +1005,7 @@ RSpec.describe CookedPostProcessor do [/quote] MD - cpp = CookedPostProcessor.new(post, disable_loading_image: true) + cpp = CookedPostProcessor.new(post, disable_dominant_color: true) cpp.post_process doc = Nokogiri::HTML5::fragment(cpp.html) @@ -1031,7 +1020,7 @@ RSpec.describe CookedPostProcessor do post = Fabricate(:post, raw: "https://discourse.org") - cpp = CookedPostProcessor.new(post, disable_loading_image: true) + cpp = CookedPostProcessor.new(post, disable_dominant_color: true) cpp.post_process doc = Nokogiri::HTML5::fragment(cpp.html) @@ -1591,7 +1580,7 @@ RSpec.describe CookedPostProcessor do RAW end - let(:cpp) { CookedPostProcessor.new(post, disable_loading_image: true) } + let(:cpp) { CookedPostProcessor.new(post, disable_dominant_color: true) } it "does remove user ids" do cpp.remove_user_ids diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index cb20d184efd..6bcc49b1aac 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -618,4 +618,76 @@ RSpec.describe Upload do expect(Upload.secure_media_url?(url)).to eq(false) end end + + describe "#dominant_color" do + let(:white_image) { Fabricate(:image_upload, color: "white") } + let(:red_image) { Fabricate(:image_upload, color: "red") } + let(:not_an_image) { + upload = Fabricate(:upload) + + file = Tempfile.new(['invalid', '.txt']) + file << "Not really an image" + file.rewind + + upload.update(url: Discourse.store.store_upload(file, upload), extension: "txt") + upload + } + let(:invalid_image) { + upload = Fabricate(:upload) + + file = Tempfile.new(['invalid', '.png']) + file << "Not really an image" + file.rewind + + upload.update(url: Discourse.store.store_upload(file, upload)) + upload + } + + it "correctly identifies and stores an image's dominant color" do + expect(white_image.dominant_color).to eq(nil) + expect(white_image.dominant_color(calculate_if_missing: true)).to eq("FFFFFF") + expect(white_image.dominant_color).to eq("FFFFFF") + + expect(red_image.dominant_color).to eq(nil) + expect(red_image.dominant_color(calculate_if_missing: true)).to eq("FF0000") + expect(red_image.dominant_color).to eq("FF0000") + end + + it "can be backfilled" do + expect(white_image.dominant_color).to eq(nil) + expect(red_image.dominant_color).to eq(nil) + + Upload.backfill_dominant_colors!(5) + + white_image.reload + red_image.reload + + expect(white_image.dominant_color).to eq("FFFFFF") + expect(red_image.dominant_color).to eq("FF0000") + end + + it "stores an empty string for non-image uploads" do + expect(not_an_image.dominant_color).to eq(nil) + expect(not_an_image.dominant_color(calculate_if_missing: true)).to eq("") + expect(not_an_image.dominant_color).to eq("") + end + + it "correctly handles invalid image files" do + expect(invalid_image.dominant_color).to eq(nil) + expect(invalid_image.dominant_color(calculate_if_missing: true)).to eq("") + expect(invalid_image.dominant_color).to eq("") + end + + it "correctly handles unparsable ImageMagick output" do + Discourse::Utils.stubs(:execute_command).returns('someinvalidoutput') + + expect(invalid_image.dominant_color).to eq(nil) + + expect { + invalid_image.dominant_color(calculate_if_missing: true) + }.to raise_error(/Calculated dominant color but unable to parse output/) + + expect(invalid_image.dominant_color).to eq(nil) + end + end end diff --git a/spec/requests/api/schemas/json/upload_create_response.json b/spec/requests/api/schemas/json/upload_create_response.json index 05555c7b9e3..32f92c5d7d9 100644 --- a/spec/requests/api/schemas/json/upload_create_response.json +++ b/spec/requests/api/schemas/json/upload_create_response.json @@ -39,6 +39,9 @@ }, "human_filesize": { "type": "string" + }, + "dominant_color": { + "type": ["string", "null"] } }, "required": [