diff options
-rw-r--r-- | actionpack/CHANGELOG.md | 6 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/redirecting.rb | 2 | ||||
-rw-r--r-- | activerecord/lib/active_record/relation/query_methods.rb | 4 | ||||
-rw-r--r-- | activerecord/test/cases/base_test.rb | 10 | ||||
-rw-r--r-- | activerecord/test/cases/locking_test.rb | 4 | ||||
-rw-r--r-- | activestorage/app/models/active_storage/blob.rb | 6 | ||||
-rw-r--r-- | activestorage/app/models/active_storage/variant.rb | 37 | ||||
-rw-r--r-- | activestorage/test/fixtures/files/icon.psd | bin | 0 -> 37441 bytes | |||
-rw-r--r-- | activestorage/test/models/variant_test.rb | 28 | ||||
-rw-r--r-- | guides/source/contributing_to_ruby_on_rails.md | 4 | ||||
-rw-r--r-- | railties/test/railties/engine_test.rb | 15 |
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 Binary files differnew file mode 100644 index 0000000000..631fceeaab --- /dev/null +++ b/activestorage/test/fixtures/files/icon.psd 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 |