-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Active storage add proxying #34477
Active storage add proxying #34477
Changes from all commits
d0b1280
177574f
31a2d2a
09f37c3
39eb31a
71b499e
7c00122
96b54a7
0b488cc
5281255
4dd0e2f
0db8f77
813b8e0
ca72a7e
73f2202
8ee4bfb
3a1bc1d
61d228d
c82b288
0f621c8
9e34b40
25429de
a55f88f
f5586e2
f016113
46f110e
3ae4936
464ed95
2d2fb66
f0c7723
331ed77
c271ee6
9aed641
e83edfb
da1df76
fe0ff8f
9676f88
b31bb85
97ebfaf
969a1ed
ec254dd
017e8ae
b932f1b
80db667
7bfb8b2
3d76291
66a4a20
93fbf27
13ac0f4
1483c17
6ef2c7e
18deb51
24244d7
0cea5a0
ba02d81
da0084c
7056d6a
cec9b0d
72edc1c
93808b7
ddbe958
8ddfa55
3d34726
16e1641
aa6cdf0
69fa76a
c53b4e6
7fb06f1
d6e69e9
a9b249a
a3c2882
a335e3b
e7979b6
290453a
4faafc0
22645e7
1e758f3
1abfd9a
1dc2188
cc8d0c3
486b0bc
f84e086
06693d4
baa1929
6ce1f1b
73639b8
64d6a28
86131e3
7c23631
7660a4f
6c1d003
21b29bc
521244f
eacde0f
e772d75
d117ab9
302923a
fb397b2
acf951a
9b7ca2c
71d8a2c
32d604c
2f15573
f35423b
cc545d6
bcb5ffe
bdd0f80
eb64ff0
d09134c
c8c67ba
68a36ad
5930a22
1938c82
fbaaca9
68f9816
e717e69
bd19891
1207c3c
ba1f16f
7819297
4accc9d
2003663
f4044b8
f67a4bc
4bd8f5f
1927754
99386a3
366ab0b
975571e
2817a38
c07d9ad
c9342c7
dc84bdd
a147010
e698761
387baf3
c08951d
293553d
c8fc088
108e344
e4eba43
491b4d5
b7cc8c7
6d732d2
176edc7
3e79f37
69260a8
7d2ad32
b09f350
ea0bda8
9ff39ac
2885507
dec20c4
517d984
3d681a9
6d99bd7
824c703
a84d081
b787c96
aea29e9
d375962
395e0b3
a7dc7dd
d3a6ba7
178260c
4d14661
49027c6
7558486
3ee6b44
ed22fd4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# frozen_string_literal: true | ||
|
||
# Proxy files through application. This avoids having a redirect and makes files easier to cache. | ||
class ActiveStorage::Blobs::ProxyController < ActiveStorage::BaseController | ||
include ActiveStorage::SetBlob | ||
include ActiveStorage::SetHeaders | ||
|
||
def show | ||
http_cache_forever public: true do | ||
set_content_headers_from @blob | ||
stream @blob | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
# frozen_string_literal: true | ||
|
||
# Proxy files through application. This avoids having a redirect and makes files easier to cache. | ||
class ActiveStorage::Representations::ProxyController < ActiveStorage::BaseController | ||
fleck marked this conversation as resolved.
Show resolved
Hide resolved
|
||
include ActiveStorage::SetBlob | ||
include ActiveStorage::SetHeaders | ||
|
||
def show | ||
http_cache_forever public: true do | ||
set_content_headers_from representation.image | ||
stream representation | ||
end | ||
end | ||
|
||
private | ||
def representation | ||
@representation ||= @blob.representation(params[:variation_key]).processed | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
# frozen_string_literal: true | ||
|
||
module ActiveStorage::SetHeaders #:nodoc: | ||
extend ActiveSupport::Concern | ||
|
||
private | ||
def set_content_headers_from(blob) | ||
response.headers["Content-Type"] = blob.content_type | ||
response.headers["Content-Disposition"] = ActionDispatch::Http::ContentDisposition.format \ | ||
disposition: params[:disposition] || "inline", filename: blob.filename.sanitized | ||
end | ||
fleck marked this conversation as resolved.
Show resolved
Hide resolved
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,7 @@ | |
class ActiveStorage::Variant | ||
attr_reader :blob, :variation | ||
delegate :service, to: :blob | ||
delegate :content_type, :filename, to: :specification | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @fleck this seems to duplicate the definition at line 128 of this same file as now I see these warnings:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That’s me. I missed the definition at line 128. I’ll fix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 57d926a. |
||
|
||
def initialize(blob, variation_or_variation_key) | ||
@blob, @variation = blob, ActiveStorage::Variation.wrap(variation_or_variation_key) | ||
|
@@ -83,6 +84,12 @@ def url(expires_in: ActiveStorage.service_urls_expire_in, disposition: :inline) | |
alias_method :service_url, :url | ||
deprecate service_url: :url | ||
|
||
# Downloads the file associated with this variant. If no block is given, the entire file is read into memory and returned. | ||
# That'll use a lot of RAM for very large files. If a block is given, then the download is streamed and yielded in chunks. | ||
def download(&block) | ||
service.download key, &block | ||
end | ||
|
||
# Returns the receiving variant. Allows ActiveStorage::Variant and ActiveStorage::Preview instances to be used interchangeably. | ||
def image | ||
self | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# frozen_string_literal: true | ||
|
||
require "test_helper" | ||
require "database/setup" | ||
|
||
class ActiveStorage::Blobs::ProxyControllerTest < ActionDispatch::IntegrationTest | ||
setup do | ||
@blob = create_file_blob filename: "racecar.jpg" | ||
end | ||
|
||
test "invalid signed ID" do | ||
get rails_service_blob_proxy_url("invalid", "racecar.jpg") | ||
assert_response :not_found | ||
end | ||
|
||
test "HTTP caching" do | ||
get rails_storage_proxy_url(@blob) | ||
assert_response :success | ||
assert_equal "max-age=3155695200, public", response.headers["Cache-Control"] | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it would be good if https://edgeguides.rubyonrails.org/active_storage_overview.html#linking-to-files mentioned this too