Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cache control headers to assets #104

Open
code-bunny opened this issue Dec 25, 2024 · 3 comments
Open

Add cache control headers to assets #104

code-bunny opened this issue Dec 25, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@code-bunny
Copy link

The assets controller as it currently works by streaming the file:

def show
  @asset = Maglev::Asset.find(resource_id)
  send_data @asset.download, filename: @asset.filename, type: @asset.content_type
end

This results in cache-control: max-age=0, private, must-revalidate

The most sensible fix is to respect the config.public_file_server.headers set by in the Rails environment.

I suggest something like below

def show
  @asset = Maglev::Asset.find(resource_id)
  send_data @asset.download, filename: @asset.filename, type: @asset.content_type

  cache_control = Rails.configuration.public_file_server.headers["cache-control"]
  if cache_control && cache_control.match(/max-age=(\d+)/)
    max_age = Regexp.last_match(1).to_i
    expires_in max_age.seconds, public: true
  end
end
@code-bunny
Copy link
Author

For now I am using the following in app/overrides/controllers/maglev/assets_controller_override.rb

Maglev::AssetsController.class_eval do
  def show
    @asset = Maglev::Asset.find(resource_id)
    send_data @asset.download, filename: @asset.filename,
                               type: @asset.content_type,
                               disposition: "inline"

    cache_control = Rails.configuration.public_file_server.headers["cache-control"]
    if cache_control && cache_control.match(/max-age=(\d+)/)
      max_age = Regexp.last_match(1).to_i
      expires_in max_age.seconds, public: true
    end
  end
end

@did did added the enhancement New feature or request label Jan 5, 2025
@did did self-assigned this Jan 5, 2025
@did
Copy link
Contributor

did commented Jan 5, 2025

You're absolutely right. I've read (again) https://guides.rubyonrails.org/active_storage_overview.html#putting-a-cdn-in-front-of-active-storage and I believe we should use it instead. It'll save your cache issue.

@code-bunny
Copy link
Author

That would indeed be the best solution and would save ballooning costs for people using S3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants