aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/CHANGELOG.md6
-rw-r--r--actionpack/lib/action_controller/metal/redirecting.rb2
-rw-r--r--activerecord/lib/active_record/relation/query_methods.rb4
-rw-r--r--activerecord/test/cases/base_test.rb10
-rw-r--r--activerecord/test/cases/locking_test.rb4
-rw-r--r--activestorage/app/models/active_storage/blob.rb6
-rw-r--r--activestorage/app/models/active_storage/variant.rb37
-rw-r--r--activestorage/test/fixtures/files/icon.psdbin0 -> 37441 bytes
-rw-r--r--activestorage/test/models/variant_test.rb28
-rw-r--r--guides/source/contributing_to_ruby_on_rails.md4
-rw-r--r--railties/test/railties/engine_test.rb15
11 files changed, 89 insertions, 27 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 384546d7b4..c75f0e83ac 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -111,9 +111,9 @@
*Rafael Mendonça França*
* Add `:allow_other_host` option to `redirect_back` method.
- When `allow_other_host` is set to `false`, the `redirect_back`
- will not allow a redirecting from a different host.
- `allow_other_host` is `true` by default.
+
+ When `allow_other_host` is set to `false`, the `redirect_back` will not allow redirecting from a
+ different host. `allow_other_host` is `true` by default.
*Tim Masliuchenko*
diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb
index 87a2e29a3f..4c2b5120eb 100644
--- a/actionpack/lib/action_controller/metal/redirecting.rb
+++ b/actionpack/lib/action_controller/metal/redirecting.rb
@@ -83,7 +83,7 @@ module ActionController
#
# ==== Options
# * <tt>:fallback_location</tt> - The default fallback location that will be used on missing +Referer+ header.
- # * <tt>:allow_other_host</tt> - Allows or disallow redirection to the host that is different to the current host
+ # * <tt>:allow_other_host</tt> - Allow or disallow redirection to the host that is different to the current host, defaults to true.
#
# All other options that can be passed to <tt>redirect_to</tt> are accepted as
# options and the behavior is identical.
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 6b3ff3d610..2056f9bb73 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -1040,8 +1040,8 @@ module ActiveRecord
def build_select(arel)
if select_values.any?
arel.project(*arel_columns(select_values.uniq))
- elsif @klass.ignored_columns.any?
- arel.project(*arel_columns(@klass.column_names.map(&:to_sym)))
+ elsif klass.ignored_columns.any?
+ arel.project(*klass.column_names.map { |field| arel_attribute(field) })
else
arel.project(table[Arel.star])
end
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 7fc4a396e4..b076b452e7 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1498,10 +1498,14 @@ class BasicsTest < ActiveRecord::TestCase
test "column names are quoted when using #from clause and model has ignored columns" do
refute_empty Developer.ignored_columns
- query = Developer.from("`developers`").to_sql
- quoted_id = Developer.connection.quote_table_name("id")
+ query = Developer.from("developers").to_sql
+ quoted_id = "#{Developer.quoted_table_name}.#{Developer.quoted_primary_key}"
- assert_match(/SELECT #{quoted_id}.* FROM `developers`/, query)
+ assert_match(/SELECT #{quoted_id}.* FROM developers/, query)
+ end
+
+ test "using table name qualified column names unless having SELECT list explicitly" do
+ assert_equal developers(:david), Developer.from("developers").joins(:shared_computers).take
end
test "protected environments by default is an array with production" do
diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
index 437a5a38a3..3701be4b11 100644
--- a/activerecord/test/cases/locking_test.rb
+++ b/activerecord/test/cases/locking_test.rb
@@ -406,7 +406,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase
assert_equal 0, car.lock_version
previously_car_updated_at = car.updated_at
- travel(1.second) do
+ travel(2.second) do
Wheel.create!(wheelable: car)
end
@@ -422,7 +422,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase
assert_equal 2, car.lock_version
previously_car_updated_at = car.updated_at
- travel(1.second) do
+ travel(2.second) do
car.wheels.first.destroy!
end
diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
index fdf17ac67e..3b48ee72af 100644
--- a/activestorage/app/models/active_storage/blob.rb
+++ b/activestorage/app/models/active_storage/blob.rb
@@ -172,11 +172,11 @@ class ActiveStorage::Blob < ActiveRecord::Base
end
- # Returns an ActiveStorage::Preview instance for a previewable blob or an ActiveStorage::Variant instance for an image blob.
+ # Returns an ActiveStorage::Preview for a previewable blob or an ActiveStorage::Variant for a variable image blob.
#
# blob.representation(resize: "100x100").processed.service_url
#
- # Raises ActiveStorage::Blob::UnrepresentableError if the receiving blob is neither an image nor previewable. Call
+ # Raises ActiveStorage::Blob::UnrepresentableError if the receiving blob is neither variable nor previewable. Call
# ActiveStorage::Blob#representable? to determine whether a blob is representable.
#
# See ActiveStorage::Blob#preview and ActiveStorage::Blob#variant for more information.
@@ -191,7 +191,7 @@ class ActiveStorage::Blob < ActiveRecord::Base
end
end
- # Returns true if the blob is variable or is previewable.
+ # Returns true if the blob is variable or previewable.
def representable?
variable? || previewable?
end
diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb
index fa5aa69bd3..8909fd628a 100644
--- a/activestorage/app/models/active_storage/variant.rb
+++ b/activestorage/app/models/active_storage/variant.rb
@@ -35,6 +35,8 @@
#
# avatar.variant(resize: "100x100", monochrome: true, flip: "-90")
class ActiveStorage::Variant
+ WEB_IMAGE_CONTENT_TYPES = %w( image/png image/jpeg image/jpg image/gif )
+
attr_reader :blob, :variation
delegate :service, to: :blob
@@ -62,7 +64,7 @@ class ActiveStorage::Variant
# for a variant that points to the ActiveStorage::VariantsController, which in turn will use this +service_call+ method
# for its redirection.
def service_url(expires_in: service.url_expires_in, disposition: :inline)
- service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename, content_type: blob.content_type
+ service.url key, expires_in: expires_in, disposition: disposition, filename: filename, content_type: content_type
end
# Returns the receiving variant. Allows ActiveStorage::Variant and ActiveStorage::Preview instances to be used interchangeably.
@@ -76,11 +78,40 @@ class ActiveStorage::Variant
end
def process
- service.upload key, transform(service.download(blob.key))
+ service.upload key, transform(blob.download)
+ end
+
+
+ def filename
+ if WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type)
+ blob.filename
+ else
+ ActiveStorage::Filename.new("#{blob.filename.base}.png")
+ end
end
+ def content_type
+ blob.content_type.presence_in(WEB_IMAGE_CONTENT_TYPES) || "image/png"
+ end
+
+
def transform(io)
+ read_image_from(io) do |image|
+ mogrify image
+ format image
+ end
+ end
+
+ def read_image_from(io, &block)
require "mini_magick"
- File.open MiniMagick::Image.read(io).tap { |image| variation.transform(image) }.path
+ File.open MiniMagick::Image.read(io).tap(&block).path
+ end
+
+ def mogrify(image)
+ variation.transform(image)
+ end
+
+ def format(image)
+ image.format("PNG") unless WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type)
end
end
diff --git a/activestorage/test/fixtures/files/icon.psd b/activestorage/test/fixtures/files/icon.psd
new file mode 100644
index 0000000000..631fceeaab
--- /dev/null
+++ b/activestorage/test/fixtures/files/icon.psd
Binary files differ
diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
index 83ebce4446..8eced41ee0 100644
--- a/activestorage/test/models/variant_test.rb
+++ b/activestorage/test/models/variant_test.rb
@@ -4,12 +4,9 @@ require "test_helper"
require "database/setup"
class ActiveStorage::VariantTest < ActiveSupport::TestCase
- setup do
- @blob = create_file_blob filename: "racecar.jpg"
- end
-
- test "resized variation" do
- variant = @blob.variant(resize: "100x100").processed
+ test "resized variation of JPEG blob" do
+ blob = create_file_blob(filename: "racecar.jpg")
+ variant = blob.variant(resize: "100x100").processed
assert_match(/racecar\.jpg/, variant.service_url)
image = read_image(variant)
@@ -17,8 +14,9 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
assert_equal 67, image.height
end
- test "resized and monochrome variation" do
- variant = @blob.variant(resize: "100x100", monochrome: true).processed
+ test "resized and monochrome variation of JPEG blob" do
+ blob = create_file_blob(filename: "racecar.jpg")
+ variant = blob.variant(resize: "100x100", monochrome: true).processed
assert_match(/racecar\.jpg/, variant.service_url)
image = read_image(variant)
@@ -27,6 +25,17 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
assert_match(/Gray/, image.colorspace)
end
+ test "resized variation of PSD blob" do
+ blob = create_file_blob(filename: "icon.psd", content_type: "image/vnd.adobe.photoshop")
+ variant = blob.variant(resize: "20x20").processed
+ assert_match(/icon\.png/, variant.service_url)
+
+ image = read_image(variant)
+ assert_equal "PNG", image.type
+ assert_equal 20, image.width
+ assert_equal 20, image.height
+ end
+
test "variation of invariable blob" do
assert_raises ActiveStorage::Blob::InvariableError do
create_file_blob(filename: "report.pdf", content_type: "application/pdf").variant(resize: "100x100")
@@ -34,7 +43,8 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
end
test "service_url doesn't grow in length despite long variant options" do
- variant = @blob.variant(font: "a" * 10_000).processed
+ blob = create_file_blob(filename: "racecar.jpg")
+ variant = blob.variant(font: "a" * 10_000).processed
assert_operator variant.service_url.length, :<, 500
end
end
diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md
index 7424818757..967c992c05 100644
--- a/guides/source/contributing_to_ruby_on_rails.md
+++ b/guides/source/contributing_to_ruby_on_rails.md
@@ -84,7 +84,9 @@ discussions new features require.
Helping to Resolve Existing Issues
----------------------------------
-As a next step beyond reporting issues, you can help the core team resolve existing issues. If you check the [issues list](https://github.com/rails/rails/issues) in GitHub Issues, you'll find lots of issues already requiring attention. What can you do for these? Quite a bit, actually:
+As a next step beyond reporting issues, you can help the core team resolve existing ones by providing feedback about them. If you are new to Rails core development, that might be a great way to walk your first steps, you'll get familiar with the code base and the processes.
+
+If you check the [issues list](https://github.com/rails/rails/issues) in GitHub Issues, you'll find lots of issues already requiring attention. What can you do for these? Quite a bit, actually:
### Verifying Bug Reports
diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb
index 4a861bff55..339a56c34f 100644
--- a/railties/test/railties/engine_test.rb
+++ b/railties/test/railties/engine_test.rb
@@ -1479,6 +1479,21 @@ YAML
assert_equal "/fruits/2/bukkits/posts", last_response.body
end
+ test "active_storage:install task works within engine" do
+ @plugin.write "Rakefile", <<-RUBY
+ APP_RAKEFILE = '#{app_path}/Rakefile'
+ load 'rails/tasks/engine.rake'
+ RUBY
+
+ Dir.chdir(@plugin.path) do
+ output = `bundle exec rake app:active_storage:install`
+ assert $?.success?, output
+
+ active_storage_migration = migrations.detect { |migration| migration.name == "CreateActiveStorageTables" }
+ assert active_storage_migration
+ end
+ end
+
private
def app
Rails.application