cache non avatar attachments for a year

lib/api/helpers/attachment_renderer.rb

@@ -40,11 +40,15 @@ module API
       # or by directly rendering the file
       #
       # @param attachment [Attachment] Attachment to be responded with.
-      # @param external_link_expires_in [ActiveSupport::Duration] Time after which link expires. Default is 5 minutes.
-      #                                                           Only applicable in case of external storage.
-      def respond_with_attachment(attachment, external_link_expires_in: nil)
+      # @param cache_seconds [integer] Time in seconds the cache headers signal the browser to cache the attachment.
+      #                                Defaults to no cache headers.
+      def respond_with_attachment(attachment, cache_seconds: nil)
+        if cache_seconds
+          set_cache_headers!(cache_seconds)
+        end
+
         if attachment.external_storage?
-          redirect attachment.external_url(expires_in: external_link_expires_in).to_s
+          redirect attachment.external_url(expires_in: cache_seconds).to_s
         else
           content_type attachment.content_type
           header['Content-Disposition'] = "#{attachment.content_disposition}; filename=#{attachment.filename}"
@@ -53,6 +57,11 @@ module API
         end
       end
 
+      def set_cache_headers!(seconds)
+        header "Cache-Control", "public, max-age=#{seconds}"
+        header "Expires", CGI.rfc1123_date(Time.now.utc + seconds)
+      end
+
       def avatar_link_expires_in
         seconds = avatar_link_expiry_seconds
 

lib/api/v3/attachments/attachments_api.rb

@@ -65,7 +65,7 @@ module API
               helpers ::API::Helpers::AttachmentRenderer
 
               get do
-                respond_with_attachment @attachment
+                respond_with_attachment @attachment, cache_seconds: 1.year.to_i
               end
             end
           end

modules/avatars/lib/api/v3/users/user_avatar_api.rb

@@ -34,21 +34,14 @@ module API
         helpers ::AvatarHelper
         helpers ::API::Helpers::AttachmentRenderer
 
-        helpers do
-          def set_cache_headers!
-            return if @user == current_user
-
-            header "Cache-Control", "public, max-age=#{avatar_link_expiry_seconds}"
-            header "Expires", CGI.rfc1123_date(Time.now.utc + avatar_link_expiry_seconds)
-          end
-        end
-
         get '/avatar' do
-          set_cache_headers!
+          cache_seconds = @user == current_user ? nil : avatar_link_expires_in
 
           if (local_avatar = local_avatar?(@user))
-            respond_with_attachment(local_avatar, external_link_expires_in: avatar_link_expires_in)
+            respond_with_attachment(local_avatar, cache_seconds: cache_seconds)
           elsif avatar_manager.gravatar_enabled?
+            set_cache_headers!(cache_seconds)
+
             redirect build_gravatar_image_url(@user)
           else
             status 404

modules/bim/app/controllers/bim/bcf/api/v2_1/viewpoints/api.rb

@@ -28,7 +28,9 @@
 # See docs/COPYRIGHT.rdoc for more details.
 #++
 
+# rubocop:disable Naming/ClassAndModuleCamelCase
 module Bim::Bcf::API::V2_1
+  # rubocop:enable Naming/ClassAndModuleCamelCase
   module Viewpoints
     class API < ::API::OpenProjectAPI
       # Avoid oj parsing numbers into BigDecimal
@@ -79,7 +81,7 @@ module Bim::Bcf::API::V2_1
             get do
               viewpoint = @issue.viewpoints.find_by!(uuid: params[:viewpoint_uuid])
               if snapshot = viewpoint.snapshot
-                respond_with_attachment snapshot
+                respond_with_attachment snapshot, cache_seconds: 1.year.to_i
               else
                 raise ActiveRecord::RecordNotFound
               end

modules/bim/spec/requests/api/bcf/v2_1/viewpoints_api_spec.rb

@@ -200,9 +200,17 @@ describe 'BCF 2.1 viewpoints resource', type: :request, content_type: :json, wit
         get path
       end
 
-      it 'responds with the attachment' do
+      it 'responds with the attachment with the appropriate content type and cache headers' do
         expect(subject.status).to eq 200
         expect(subject.headers['Content-Type']).to eq 'image/jpeg'
+
+        expect(subject.headers["Cache-Control"]).to eq "public, max-age=#{1.year.to_i}"
+        expect(subject.headers["Expires"]).to be_present
+
+        expires_time = Time.parse response.headers["Expires"]
+
+        expect(expires_time < Time.now.utc + 1.year.to_i).to be_truthy
+        expect(expires_time > Time.now.utc + 1.year.to_i - 60).to be_truthy
       end
     end
 

spec/requests/api/v3/attachments/attachment_resource_shared_examples.rb

@@ -270,12 +270,20 @@ shared_examples 'an APIv3 attachment resource', type: :request, content_type: :j
           expect(subject.status).to eq 200
         end
 
-        it 'has the necessary headers' do
+        it 'has the necessary headers for content and caching' do
           expect(subject.headers['Content-Disposition'])
             .to eql content_disposition
 
           expect(subject.headers['Content-Type'])
             .to eql mock_file.content_type
+
+          expect(subject.headers["Cache-Control"]).to eq "public, max-age=#{1.year.to_i}"
+          expect(subject.headers["Expires"]).to be_present
+
+          expires_time = Time.parse response.headers["Expires"]
+
+          expect(expires_time < Time.now.utc + 1.year.to_i).to be_truthy
+          expect(expires_time > Time.now.utc + 1.year.to_i - 60).to be_truthy
         end
 
         it 'sends the file in binary' do
@@ -323,6 +331,14 @@ shared_examples 'an APIv3 attachment resource', type: :request, content_type: :j
           expect(subject.status).to eq 302
           expect(subject.headers['Location'])
             .to eql external_url
+
+          expect(subject.headers["Cache-Control"]).to eq "public, max-age=#{1.year.to_i}"
+          expect(subject.headers["Expires"]).to be_present
+
+          expires_time = Time.parse response.headers["Expires"]
+
+          expect(expires_time < Time.now.utc + 1.year.to_i).to be_truthy
+          expect(expires_time > Time.now.utc + 1.year.to_i - 60).to be_truthy
         end
       end
     end