diff options
15 files changed, 66 insertions, 45 deletions
diff --git a/actioncable/test/connection/multiple_identifiers_test.rb b/actioncable/test/connection/multiple_identifiers_test.rb index 230433a110..7f90cb3876 100644 --- a/actioncable/test/connection/multiple_identifiers_test.rb +++ b/actioncable/test/connection/multiple_identifiers_test.rb @@ -36,8 +36,4 @@ class ActionCable::Connection::MultipleIdentifiersTest < ActionCable::TestCase @connection.process @connection.send :handle_open end - - def close_connection - @connection.send :handle_close - end end diff --git a/actioncable/test/connection/string_identifier_test.rb b/actioncable/test/connection/string_identifier_test.rb index 6387fb792c..4cb58e7fd0 100644 --- a/actioncable/test/connection/string_identifier_test.rb +++ b/actioncable/test/connection/string_identifier_test.rb @@ -38,8 +38,4 @@ class ActionCable::Connection::StringIdentifierTest < ActionCable::TestCase @connection.process @connection.send :on_open end - - def close_connection - @connection.send :on_close - end end diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index f548fc24ed..f2edcb750c 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -355,7 +355,7 @@ module ActionView # finds the options and details and extracts them. The method also contains # logic that handles the type of object passed in as the partial. # - # If +options[:partial]+ is a string, then the +@path+ instance variable is + # If +options[:partial]+ is a string, then the <tt>@path</tt> instance variable is # set to that string. Otherwise, the +options[:partial]+ object must # respond to +to_partial_path+ in order to setup the path. def setup(context, options, block) diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 9b0b50977d..3d79e540b8 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -68,11 +68,11 @@ module ActiveRecord foreign_key = join_keys.foreign_key value = transform_value(owner[foreign_key]) - scope = scope.where(table.name => { key => value }) + scope = apply_scope(scope, table, key, value) if reflection.type polymorphic_type = transform_value(owner.class.base_class.name) - scope = scope.where(table.name => { reflection.type => polymorphic_type }) + scope = apply_scope(scope, table, reflection.type, polymorphic_type) end scope @@ -91,10 +91,10 @@ module ActiveRecord if reflection.type value = transform_value(next_reflection.klass.base_class.name) - scope = scope.where(table.name => { reflection.type => value }) + scope = apply_scope(scope, table, reflection.type, value) end - scope = scope.joins(join(foreign_table, constraint)) + scope.joins!(join(foreign_table, constraint)) end class ReflectionProxy < SimpleDelegator # :nodoc: @@ -165,6 +165,14 @@ module ActiveRecord scope end + def apply_scope(scope, table, key, value) + if scope.table == table + scope.where!(key => value) + else + scope.where!(table.name => { key => value }) + end + end + def eval_scope(reflection, table, scope, owner) reflection.build_scope(table).instance_exec(owner, &scope) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 11b5f48dc7..0759f4d2b3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -270,7 +270,7 @@ module ActiveRecord # Connections must be leased while holding the main pool mutex. This is # an internal subclass that also +.leases+ returned connections while # still in queue's critical section (queue synchronizes with the same - # +@lock+ as the main pool) so that a returned connection is already + # <tt>@lock</tt> as the main pool) so that a returned connection is already # leased and there is no need to re-enter synchronized block. class ConnectionLeasingQueue < Queue # :nodoc: include BiasableQueue @@ -338,7 +338,7 @@ module ActiveRecord # then that +thread+ does indeed own that +conn+. However, an absence of a such # mapping does not mean that the +thread+ doesn't own the said connection. In # that case +conn.owner+ attr should be consulted. - # Access and modification of +@thread_cached_conns+ does not require + # Access and modification of <tt>@thread_cached_conns</tt> does not require # synchronization. @thread_cached_conns = Concurrent::Map.new(initial_capacity: @size) @@ -737,10 +737,10 @@ module ActiveRecord # Implementation detail: the connection returned by +acquire_connection+ # will already be "+connection.lease+ -ed" to the current thread. def acquire_connection(checkout_timeout) - # NOTE: we rely on +@available.poll+ and +try_to_checkout_new_connection+ to + # NOTE: we rely on <tt>@available.poll</tt> and +try_to_checkout_new_connection+ to # +conn.lease+ the returned connection (and to do this in a +synchronized+ # section). This is not the cleanest implementation, as ideally we would - # <tt>synchronize { conn.lease }</tt> in this method, but by leaving it to +@available.poll+ + # <tt>synchronize { conn.lease }</tt> in this method, but by leaving it to <tt>@available.poll</tt> # and +try_to_checkout_new_connection+ we can piggyback on +synchronize+ sections # of the said methods and avoid an additional +synchronize+ overhead. if conn = @available.poll || try_to_checkout_new_connection @@ -764,7 +764,7 @@ module ActiveRecord end end - # If the pool is not at a +@size+ limit, establish new connection. Connecting + # If the pool is not at a <tt>@size</tt> limit, establish new connection. Connecting # to the DB is done outside main synchronized section. #-- # Implementation constraint: a newly established connection returned by this diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index f91f0cdf12..b2f5e39e09 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -472,7 +472,7 @@ module ActiveRecord # if it's associated with a transaction, then the state of the Active Record # object will be updated to reflect the current state of the transaction. # - # The +@transaction_state+ variable stores the states of the associated + # The <tt>@transaction_state</tt> variable stores the states of the associated # transaction. This relies on the fact that a transaction can only be in # one rollback or commit (otherwise a list of states would be required). # Each Active Record object inside of a transaction carries that transaction's diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 05420a4240..47749c07d2 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -192,6 +192,7 @@ ActiveRecord::Schema.define do t.string :resource_type t.integer :developer_id t.datetime :deleted_at + t.integer :comments end create_table :companies, force: true do |t| diff --git a/activestorage/README.md b/activestorage/README.md index 957adc05c3..a86a04421e 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -88,7 +88,7 @@ Variation of image attachment: 1. Run `rails activestorage:install` to create needed directories, migrations, and configuration. 2. Optional: Add `gem "aws-sdk", "~> 2"` to your Gemfile if you want to use AWS S3. 3. Optional: Add `gem "google-cloud-storage", "~> 1.3"` to your Gemfile if you want to use Google Cloud Storage. -4. Optional: Add `gem "azure-storage"` to your Gemfile if you want to use Microsoft Azure. +4. Optional: Add `gem "azure-storage"` to your Gemfile if you want to use Microsoft Azure Storage. 5. Optional: Add `gem "mini_magick"` to your Gemfile if you want to use variants. ## Direct uploads diff --git a/activestorage/app/controllers/active_storage/blobs_controller.rb b/activestorage/app/controllers/active_storage/blobs_controller.rb index 05af29f8b2..cff88bd488 100644 --- a/activestorage/app/controllers/active_storage/blobs_controller.rb +++ b/activestorage/app/controllers/active_storage/blobs_controller.rb @@ -5,6 +5,7 @@ class ActiveStorage::BlobsController < ActionController::Base def show if blob = find_signed_blob + expires_in 5.minutes # service_url defaults to 5 minutes redirect_to blob.service_url(disposition: disposition_param) else head :not_found diff --git a/activestorage/app/controllers/active_storage/variants_controller.rb b/activestorage/app/controllers/active_storage/variants_controller.rb index 994c57aafd..b72b0ff7f5 100644 --- a/activestorage/app/controllers/active_storage/variants_controller.rb +++ b/activestorage/app/controllers/active_storage/variants_controller.rb @@ -5,6 +5,7 @@ class ActiveStorage::VariantsController < ActionController::Base def show if blob = find_signed_blob + expires_in 5.minutes # service_url defaults to 5 minutes redirect_to ActiveStorage::Variant.new(blob, decoded_variation).processed.service_url(disposition: disposition_param) else head :not_found diff --git a/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb b/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb index 6eab7e0fa0..2c7e3c5bc6 100644 --- a/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb +++ b/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb @@ -1,24 +1,24 @@ class CreateActiveStorageTables < ActiveRecord::Migration[5.1] def change create_table :active_storage_blobs do |t| - t.string :key - t.string :filename + t.string :key, null: false + t.string :filename, null: false t.string :content_type t.text :metadata - t.integer :byte_size - t.string :checksum - t.datetime :created_at + t.integer :byte_size, null: false + t.string :checksum, null: false + t.datetime :created_at, null: false t.index [ :key ], unique: true end create_table :active_storage_attachments do |t| - t.string :name - t.string :record_type - t.integer :record_id - t.integer :blob_id + t.string :name, null: false + t.string :record_type, null: false + t.integer :record_id, null: false + t.integer :blob_id, null: false - t.datetime :created_at + t.datetime :created_at, null: false t.index :blob_id t.index [ :record_type, :record_id, :name, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb index 6bd0240606..eb877f10c0 100644 --- a/activestorage/lib/active_storage/attached/macros.rb +++ b/activestorage/lib/active_storage/attached/macros.rb @@ -22,13 +22,11 @@ module ActiveStorage # If the +:dependent+ option isn't set, the attachment will be purged # (i.e. destroyed) whenever the record is destroyed. def has_one_attached(name, dependent: :purge_later) - define_method(name) do - if instance_variable_defined?("@active_storage_attached_#{name}") - instance_variable_get("@active_storage_attached_#{name}") - else - instance_variable_set("@active_storage_attached_#{name}", ActiveStorage::Attached::One.new(name, self)) + class_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{name} + @active_storage_attached_#{name} ||= ActiveStorage::Attached::One.new("#{name}", self) end - end + CODE has_one :"#{name}_attachment", -> { where(name: name) }, class_name: "ActiveStorage::Attachment", as: :record has_one :"#{name}_blob", through: :"#{name}_attachment", class_name: "ActiveStorage::Blob", source: :blob @@ -63,13 +61,11 @@ module ActiveStorage # If the +:dependent+ option isn't set, all the attachments will be purged # (i.e. destroyed) whenever the record is destroyed. def has_many_attached(name, dependent: :purge_later) - define_method(name) do - if instance_variable_defined?("@active_storage_attached_#{name}") - instance_variable_get("@active_storage_attached_#{name}") - else - instance_variable_set("@active_storage_attached_#{name}", ActiveStorage::Attached::Many.new(name, self)) + class_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{name} + @active_storage_attached_#{name} ||= ActiveStorage::Attached::Many.new("#{name}", self) end - end + CODE has_many :"#{name}_attachments", -> { where(name: name) }, as: :record, class_name: "ActiveStorage::Attachment" has_many :"#{name}_blobs", through: :"#{name}_attachments", class_name: "ActiveStorage::Blob", source: :blob diff --git a/activestorage/test/controllers/blobs_controller_test.rb b/activestorage/test/controllers/blobs_controller_test.rb new file mode 100644 index 0000000000..5ec353889c --- /dev/null +++ b/activestorage/test/controllers/blobs_controller_test.rb @@ -0,0 +1,15 @@ +require "test_helper" +require "database/setup" + +class ActiveStorage::BlobsControllerTest < ActionDispatch::IntegrationTest + setup do + @blob = create_image_blob filename: "racecar.jpg" + end + + test "showing blob utilizes browser caching" do + get rails_blob_url(@blob) + + assert_redirected_to(/racecar.jpg/) + assert_equal "max-age=300, private", @response.headers["Cache-Control"] + end +end diff --git a/guides/source/generators.md b/guides/source/generators.md index be1be75e7a..389224d908 100644 --- a/guides/source/generators.md +++ b/guides/source/generators.md @@ -627,7 +627,7 @@ This method also takes a block: ```ruby lib "super_special.rb" do - puts "Super special!" + "puts 'Super special!'" end ``` @@ -636,7 +636,7 @@ end Creates a Rake file in the `lib/tasks` directory of the application. ```ruby -rakefile "test.rake", "hello there" +rakefile "test.rake", 'task(:hello) { puts "Hello, there" }' ``` This method also takes a block: diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 0070527c7b..ff73014046 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -395,12 +395,13 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_default_frameworks_are_required_when_others_are_removed - run_generator [destination_root, "--skip-active-record", "--skip-action-mailer", "--skip-action-cable", "--skip-sprockets"] + run_generator [destination_root, "--skip-active-record", "--skip-action-mailer", "--skip-action-cable", "--skip-sprockets", "--skip-test"] assert_file "config/application.rb", /require\s+["']rails["']/ assert_file "config/application.rb", /require\s+["']active_model\/railtie["']/ assert_file "config/application.rb", /require\s+["']active_job\/railtie["']/ assert_file "config/application.rb", /require\s+["']action_controller\/railtie["']/ assert_file "config/application.rb", /require\s+["']action_view\/railtie["']/ + assert_file "config/application.rb", /require\s+["']active_storage\/engine["']/ end def test_generator_defaults_to_puma_version @@ -486,6 +487,7 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "config/application.rb", /#\s+require\s+["']action_cable\/engine["']/ assert_no_file "config/cable.yml" assert_no_file "app/assets/javascripts/cable.js" + assert_no_directory "app/assets/javascripts/channels" assert_no_directory "app/channels" assert_file "Gemfile" do |content| assert_no_match(/redis/, content) @@ -592,6 +594,11 @@ class AppGeneratorTest < Rails::Generators::TestCase run_generator([destination_root]) assert_file "package.json", /dependencies/ assert_file "config/initializers/assets.rb", /node_modules/ + + assert_file ".gitignore" do |content| + assert_match(/node_modules/, content) + assert_match(/yarn-error\.log/, content) + end end def test_generator_for_yarn_skipped |