Merge branch 'dynamic-image-resizer' into 'master'

Add `send-scaled-img:` handler for Workhorse

See merge request gitlab-org/gitlab!37342

app/controllers/concerns/send_file_upload.rb

@@ -18,7 +18,11 @@ module SendFileUpload
       send_params.merge!(filename: attachment, disposition: disposition)
     end
 
-    if file_upload.file_storage?
+    if image_scaling_request?(file_upload)
+      location = file_upload.file_storage? ? file_upload.path : file_upload.url
+      headers.store(*Gitlab::Workhorse.send_scaled_image(location, params[:width].to_i))
+      head :ok
+    elsif file_upload.file_storage?
       send_file file_upload.path, send_params
     elsif file_upload.class.proxy_download_enabled? || proxy
       headers.store(*Gitlab::Workhorse.send_url(file_upload.url(**redirect_params)))
@@ -37,4 +41,19 @@ module SendFileUpload
       "application/octet-stream"
     end
   end
+
+  private
+
+  def image_scaling_request?(file_upload)
+    avatar_image_upload?(file_upload) && valid_image_scaling_width? && current_user &&
+      Feature.enabled?(:dynamic_image_resizing, current_user)
+  end
+
+  def avatar_image_upload?(file_upload)
+    file_upload.try(:image?) && file_upload.try(:mounted_as)&.to_sym == :avatar
+  end
+
+  def valid_image_scaling_width?
+    Avatarable::ALLOWED_IMAGE_SCALER_WIDTHS.include?(params[:width]&.to_i)
+  end
 end

app/models/concerns/avatarable.rb

@@ -3,6 +3,17 @@
 module Avatarable
   extend ActiveSupport::Concern
 
+  ALLOWED_IMAGE_SCALER_WIDTHS = [
+    400,
+    200,
+    64,
+    48,
+    40,
+    26,
+    20,
+    16
+  ].freeze
+
   included do
     prepend ShadowMethods
     include ObjectStorage::BackgroundMove

config/feature_flags/development/dynamic_image_resizing.yml

+---
+name: dynamic_image_resizing
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37342
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/233704
+group: group::memory
+type: development
+default_enabled: false
\ No newline at end of file

lib/gitlab/workhorse.rb

@@ -156,6 +156,18 @@ module Gitlab
         ]
       end
 
+      def send_scaled_image(location, width)
+        params = {
+          'Location' => location,
+          'Width' => width
+        }
+
+        [
+          SEND_DATA_HEADER,
+          "send-scaled-img:#{encode(params)}"
+        ]
+      end
+
       def channel_websocket(channel)
         details = {
           'Channel' => {

spec/controllers/concerns/send_file_upload_spec.rb

@@ -21,6 +21,12 @@ RSpec.describe SendFileUpload do
   let(:controller_class) do
     Class.new do
       include SendFileUpload
+
+      def params
+        {}
+      end
+
+      def current_user; end
     end
   end
 
@@ -42,6 +48,89 @@ RSpec.describe SendFileUpload do
       FileUtils.rm_f(temp_file)
     end
 
+    shared_examples 'handles image resize requests' do
+      let(:headers) { double }
+
+      before do
+        allow(uploader).to receive(:image?).and_return(true)
+        allow(uploader).to receive(:mounted_as).and_return(:avatar)
+
+        allow(controller).to receive(:headers).and_return(headers)
+        # both of these are valid cases, depending on whether we are dealing with
+        # local or remote files
+        allow(controller).to receive(:send_file)
+        allow(controller).to receive(:redirect_to)
+      end
+
+      context 'when feature is enabled for current user' do
+        let(:user) { build(:user) }
+
+        before do
+          stub_feature_flags(dynamic_image_resizing: user)
+          allow(controller).to receive(:current_user).and_return(user)
+        end
+
+        context 'with valid width parameter' do
+          it 'renders OK with workhorse command header' do
+            expect(controller).not_to receive(:send_file)
+            expect(controller).to receive(:params).at_least(:once).and_return(width: '64')
+            expect(controller).to receive(:head).with(:ok)
+            expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
+
+            subject
+          end
+        end
+
+        context 'with missing width parameter' do
+          it 'does not write workhorse command header' do
+            expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
+
+            subject
+          end
+        end
+
+        context 'with invalid width parameter' do
+          it 'does not write workhorse command header' do
+            expect(controller).to receive(:params).at_least(:once).and_return(width: 'not a number')
+            expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
+
+            subject
+          end
+        end
+
+        context 'with width that is not allowed' do
+          it 'does not write workhorse command header' do
+            expect(controller).to receive(:params).at_least(:once).and_return(width: '63')
+            expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
+
+            subject
+          end
+        end
+
+        context 'when image file is not an avatar' do
+          it 'does not write workhorse command header' do
+            expect(uploader).to receive(:mounted_as).and_return(nil) # FileUploader is not mounted
+            expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
+
+            subject
+          end
+        end
+      end
+
+      context 'when feature is disabled' do
+        before do
+          stub_feature_flags(dynamic_image_resizing: false)
+        end
+
+        it 'does not write workhorse command header' do
+          expect(controller).to receive(:params).at_least(:once).and_return(width: '64')
+          expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
+
+          subject
+        end
+      end
+    end
+
     context 'when local file is used' do
       before do
         uploader.store!(temp_file)
@@ -52,6 +141,8 @@ RSpec.describe SendFileUpload do
 
         subject
       end
+
+      it_behaves_like 'handles image resize requests'
     end
 
     context 'with inline image' do
@@ -155,6 +246,8 @@ RSpec.describe SendFileUpload do
           it_behaves_like 'proxied file'
         end
       end
+
+      it_behaves_like 'handles image resize requests'
     end
   end
 end

spec/lib/gitlab/workhorse_spec.rb

@@ -421,6 +421,24 @@ RSpec.describe Gitlab::Workhorse do
     end
   end
 
+  describe '.send_scaled_image' do
+    let(:location) { 'http://example.com/avatar.png' }
+    let(:width) { '150' }
+
+    subject { described_class.send_scaled_image(location, width) }
+
+    it 'sets the header correctly' do
+      key, command, params = decode_workhorse_header(subject)
+
+      expect(key).to eq("Gitlab-Workhorse-Send-Data")
+      expect(command).to eq("send-scaled-img")
+      expect(params).to eq({
+        'Location' => location,
+        'Width' => width
+      }.deep_stringify_keys)
+    end
+  end
+
   describe '.send_git_snapshot' do
     let(:url) { 'http://example.com' }