Skip to content

Commit

Permalink
Allow configuration to conditionally prefer Cloudflare over Cloudinary (
Browse files Browse the repository at this point in the history
forem#20539)

* Allow configuration to conditionally prefer Cloudflare over Cloudinary

* Update app/services/images/optimizer.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Fix some lints

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
benhalpern and github-actions[bot] authored Jan 19, 2024
1 parent c11160d commit aab0a19
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 1 deletion.
10 changes: 9 additions & 1 deletion app/services/images/optimizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def self.call(img_src, **kwargs)

if imgproxy_enabled?
imgproxy(img_src, **kwargs)
elsif cloudinary_enabled?
elsif cloudinary_enabled? && !cloudflare_contextually_preferred?(img_src)
cloudinary(img_src, **kwargs)
elsif cloudflare_enabled?
cloudflare(img_src, **kwargs)
Expand Down Expand Up @@ -129,5 +129,13 @@ def self.extract_suffix_url(full_url)
match = uri.path.match(%r{https?.+})
CGI.unescape(match[0]) if match
end

# This is a feature-flagged Cloudflare preference for hosted images only — works specifically with S3-hosted image sources.
def self.cloudflare_contextually_preferred?(img_src)
return false unless cloudflare_enabled?
return false unless FeatureFlag.enabled?(:cloudflare_preferred_for_hosted_images)

img_src&.start_with?("https://#{ApplicationConfig['AWS_BUCKET_NAME']}.s3.amazonaws.com")
end
end
end
66 changes: 66 additions & 0 deletions spec/services/images/optimizer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,25 @@
expect(described_class).not_to have_received(:cloudinary)
expect(described_class).to have_received(:imgproxy)
end

context "when cloudflare is contextually preferred" do
before do
allow(described_class).to receive(:cloudflare_contextually_preferred?).and_return(true)
allow(described_class).to receive(:cloudflare_enabled?).and_return(true)
allow(described_class).to receive(:cloudflare)
end

it "prefers cloudflare over other services" do
described_class.call(image_url)
expect(described_class).to have_received(:cloudflare)
expect(described_class).not_to have_received(:cloudinary)
expect(described_class).not_to have_received(:imgproxy)
end
end
end

describe "#cloudinary", :cloudinary do

it "performs exactly like cl_image_path" do
cloudinary_url = cl_image_path(image_url,
type: "fetch",
Expand Down Expand Up @@ -118,6 +134,7 @@
end

it "generates correct crop when CROP_WITH_IMAGGA_SCALE is set" do
allow(ApplicationConfig).to receive(:[]).with("CLOUDFLARE_IMAGES_DOMAIN").and_return(nil)
allow(ApplicationConfig).to receive(:[]).with("CROP_WITH_IMAGGA_SCALE").and_return("true")
cloudinary_url = cl_image_path(image_url,
type: "fetch",
Expand All @@ -130,6 +147,7 @@
end

it "generates correct crop when CROP_WITH_IMAGGA_SCALE is set but never_imagga: true is passed" do
allow(ApplicationConfig).to receive(:[]).with("CLOUDFLARE_IMAGES_DOMAIN").and_return(nil)
allow(ApplicationConfig).to receive(:[]).with("CROP_WITH_IMAGGA_SCALE").and_return("true")
cl_url = cl_image_path(image_url,
type: "fetch",
Expand Down Expand Up @@ -312,4 +330,52 @@
expect(described_class.translate_cloudinary_options(options)).to include(resizing_type: "fit")
end
end

describe "#cloudflare_contextually_preferred?" do
let(:aws_bucket_name) { "mybucket" }
let(:hosted_image_url) { "https://#{aws_bucket_name}.s3.amazonaws.com/path/to/image.jpg" }
let(:non_hosted_image_url) { "https://example.com/path/to/image.jpg" }

before do
allow(ApplicationConfig).to receive(:[]).with("AWS_BUCKET_NAME").and_return(aws_bucket_name)
end

context "when cloudflare is enabled and feature flag is on" do
before do
allow(described_class).to receive(:cloudflare_enabled?).and_return(true)
allow(FeatureFlag).to receive(:enabled?).with(:cloudflare_preferred_for_hosted_images).and_return(true)
end

it "returns true for hosted images" do
expect(described_class.cloudflare_contextually_preferred?(hosted_image_url)).to be(true)
end

it "returns false for non-hosted images" do
expect(described_class.cloudflare_contextually_preferred?(non_hosted_image_url)).to be(false)
end
end

context "when cloudflare is disabled" do
before do
allow(described_class).to receive(:cloudflare_enabled?).and_return(false)
end

it "returns false regardless of image source" do
expect(described_class.cloudflare_contextually_preferred?(hosted_image_url)).to be(false)
expect(described_class.cloudflare_contextually_preferred?(non_hosted_image_url)).to be(false)
end
end

context "when feature flag is off" do
before do
allow(described_class).to receive(:cloudflare_enabled?).and_return(true)
allow(FeatureFlag).to receive(:enabled?).with(:cloudflare_preferred_for_hosted_images).and_return(false)
end

it "returns false regardless of image source" do
expect(described_class.cloudflare_contextually_preferred?(hosted_image_url)).to be(false)
expect(described_class.cloudflare_contextually_preferred?(non_hosted_image_url)).to be(false)
end
end
end
end

0 comments on commit aab0a19

Please sign in to comment.