diff options
53 files changed, 394 insertions, 208 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c1c511a65c..b39d28ca31 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,13 @@ +* Ensure `sum` honors `distinct` on `has_many :through` associations + + Fixes #16791. + + *Aaron Wortham* + +* Add `binary` fixture helper method. + + *Atsushi Yoshida* + * When using `Relation#or`, extract the common conditions and put them before the OR condition. *Maxime Handfield Lapointe* diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 38e7ba39d3..ceedf150e3 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -55,13 +55,16 @@ module ActiveRecord pk_type = reflection.association_primary_key_type ids = Array(ids).reject(&:blank?) ids.map! { |i| pk_type.cast(i) } - records = klass.where(reflection.association_primary_key => ids).index_by do |r| - r.send(reflection.association_primary_key) + + primary_key = reflection.association_primary_key + records = klass.where(primary_key => ids).index_by do |r| + r.public_send(primary_key) end.values_at(*ids).compact + if records.size != ids.size - found_ids = records.map { |record| record.send(reflection.association_primary_key) } + found_ids = records.map { |record| record.public_send(primary_key) } not_found_ids = ids - found_ids - klass.all.raise_record_not_found_exception!(ids, records.size, ids.size, reflection.association_primary_key, not_found_ids) + klass.all.raise_record_not_found_exception!(ids, records.size, ids.size, primary_key, not_found_ids) else replace(records) end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 5ba03c555a..4915a37f06 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -116,18 +116,8 @@ module ActiveRecord @reflection_scope ||= reflection.scope_for(klass) end - def klass_scope - current_scope = klass.current_scope - - if current_scope && current_scope.empty_scope? - klass.unscoped - else - klass.default_scoped - end - end - def build_scope - scope = klass_scope + scope = klass.scope_for_association if reflection.type scope.where!(reflection.type => model.base_class.sti_name) diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index 76237c4a0c..cba565448f 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -15,7 +15,7 @@ module ActiveRecord def target_scope scope = super reflection.chain.drop(1).each do |reflection| - relation = reflection.klass.all + relation = reflection.klass.scope_for_association scope.merge!( relation.except(:select, :create_with, :includes, :preload, :joins, :eager_load) ) diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 48d33e6744..c622df132e 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -62,12 +62,6 @@ module ActiveRecord clear_mutation_trackers end - def write_attribute_without_type_cast(attr_name, *) # :nodoc: - result = super - clear_attribute_change(attr_name) - result - end - def clear_attribute_changes(attr_names) # :nodoc: super attr_names.each do |attr_name| @@ -183,6 +177,11 @@ module ActiveRecord end private + def write_attribute_without_type_cast(attr_name, _) + result = super + clear_attribute_change(attr_name) + result + end def mutation_tracker unless defined?(@mutation_tracker) diff --git a/activerecord/lib/active_record/attribute_methods/serialization.rb b/activerecord/lib/active_record/attribute_methods/serialization.rb index acd47629dd..ebc2baed34 100644 --- a/activerecord/lib/active_record/attribute_methods/serialization.rb +++ b/activerecord/lib/active_record/attribute_methods/serialization.rb @@ -70,7 +70,7 @@ module ActiveRecord end decorate_attribute_type(attr_name, :serialize) do |type| - if type_incompatible_with_serialize?(type) + if type_incompatible_with_serialize?(type, class_name_or_coder) raise ColumnNotSerializableError.new(attr_name, type) end @@ -80,12 +80,9 @@ module ActiveRecord private - def type_incompatible_with_serialize?(type) - type.is_a?(ActiveRecord::Type::Json) || - ( - defined?(ActiveRecord::ConnectionAdapters::PostgreSQL) && - type.is_a?(ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Array) - ) + def type_incompatible_with_serialize?(type, class_name) + type.is_a?(ActiveRecord::Type::Json) && class_name == ::JSON || + type.respond_to?(:type_cast_array, true) && class_name == ::Array end end end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index ef302fc0a0..4940e122f4 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -1065,6 +1065,10 @@ class ActiveRecord::FixtureSet::RenderContext # :nodoc: def get_binding binding() end + + def binary(path) + %(!!binary "#{Base64.strict_encode64(File.read(path))}") + end end end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 7f1601805c..b847933b2e 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -221,13 +221,8 @@ module ActiveRecord end def klass_join_scope(table, predicate_builder) # :nodoc: - current_scope = klass.current_scope - - if current_scope && current_scope.empty_scope? - build_scope(table, predicate_builder) - else - klass.default_scoped(build_scope(table, predicate_builder)) - end + relation = build_scope(table, predicate_builder) + klass.scope_for_association(relation) end def constraints @@ -329,6 +324,10 @@ module ActiveRecord def join_pk(_) foreign_key end + + def primary_key(klass) + klass.primary_key || raise(UnknownPrimaryKey.new(klass)) + end end # Base class for AggregateReflection and AssociationReflection. Objects of @@ -697,10 +696,6 @@ module ActiveRecord def derive_join_table ModelSchema.derive_join_table_name active_record.table_name, klass.table_name end - - def primary_key(klass) - klass.primary_key || raise(UnknownPrimaryKey.new(klass)) - end end class HasManyReflection < AssociationReflection # :nodoc: @@ -715,6 +710,10 @@ module ActiveRecord Associations::HasManyAssociation end end + + def association_primary_key(klass = nil) + primary_key(klass || self.klass) + end end class HasOneReflection < AssociationReflection # :nodoc: @@ -1016,10 +1015,6 @@ module ActiveRecord end end - def primary_key(klass) - klass.primary_key || raise(UnknownPrimaryKey.new(klass)) - end - def inverse_name; delegate_reflection.send(:inverse_name); end def derive_class_name diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index 141ad176ea..fa19c679cf 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -47,7 +47,12 @@ module ActiveRecord # handle from 10000 and beyond by setting the +:start+ and +:finish+ # option on each worker. # - # # Let's process from record 10_000 on. + # # In worker 1, let's process until 9999 records. + # Person.find_each(finish: 9_999) do |person| + # person.party_all_night! + # end + # + # # In worker 2, let's process from record 10_000 and onwards. # Person.find_each(start: 10_000) do |person| # person.party_all_night! # end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index d3b5be6bce..42d43224fa 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -259,6 +259,9 @@ module ActiveRecord column = aggregate_column(column_name) select_value = operation_over_aggregate_column(column, operation, distinct) + if operation == "sum" && distinct + select_value.distinct = true + end column_alias = select_value.alias column_alias ||= @klass.connection.column_name_for_operation(operation, select_value) diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index 43cce19c1f..6fa096c1fe 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -31,6 +31,16 @@ module ActiveRecord end end + def scope_for_association(scope = relation) # :nodoc: + current_scope = self.current_scope + + if current_scope && current_scope.empty_scope? + scope + else + default_scoped(scope) + end + end + def default_scoped(scope = relation) # :nodoc: build_default_scope(scope) || scope end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 2677fade18..baeb653c61 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -8,6 +8,10 @@ module ActiveRecord raise ArgumentError, "#{options[:conditions]} was passed as :conditions but is not callable. " \ "Pass a callable instead: `conditions: -> { where(approved: true) }`" end + unless Array(options[:scope]).all? { |scope| scope.respond_to?(:to_sym) } + raise ArgumentError, "#{options[:scope]} is not supported format for :scope option. " \ + "Pass a symbol or an array of symbols instead: `scope: :user_id`" + end super({ case_sensitive: true }.merge!(options)) @klass = options[:class] end diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 08b17f37e2..0e9e86f425 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -47,7 +47,7 @@ class PostgresqlArrayTest < ActiveRecord::PostgreSQLTestCase assert ratings_column.array? end - def test_not_compatible_with_serialize + def test_not_compatible_with_serialize_array new_klass = Class.new(PgArray) do serialize :tags, Array end @@ -56,6 +56,30 @@ class PostgresqlArrayTest < ActiveRecord::PostgreSQLTestCase end end + class MyTags + def initialize(tags); @tags = tags end + def to_a; @tags end + def self.load(tags); new(tags) end + def self.dump(object); object.to_a end + end + + def test_array_with_serialized_attributes + new_klass = Class.new(PgArray) do + serialize :tags, MyTags + end + + new_klass.create!(tags: MyTags.new(["one", "two"])) + record = new_klass.first + + assert_instance_of MyTags, record.tags + assert_equal ["one", "two"], record.tags.to_a + + record.tags = MyTags.new(["three", "four"]) + record.save! + + assert_equal ["three", "four"], record.reload.tags.to_a + end + def test_default @connection.add_column "pg_arrays", "score", :integer, array: true, default: [4, 4, 2] PgArray.reset_column_information diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index cedb621b4f..383c7314e1 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -63,7 +63,7 @@ class HasManyAssociationsTestPrimaryKeys < ActiveRecord::TestCase assert_equal 2, subscriber.subscriptions.size end - assert_equal subscriber.subscriptions, Subscription.where(subscriber_id: "webster132") + assert_equal Subscription.where(subscriber_id: "webster132"), subscriber.subscriptions end def test_association_primary_key_on_new_record_should_fetch_with_query @@ -74,12 +74,23 @@ class HasManyAssociationsTestPrimaryKeys < ActiveRecord::TestCase assert_equal 1, author.essays.size end - assert_equal author.essays, Essay.where(writer_id: "David") + assert_equal Essay.where(writer_id: "David"), author.essays end def test_has_many_custom_primary_key david = authors(:david) - assert_equal david.essays, Essay.where(writer_id: "David") + assert_equal Essay.where(writer_id: "David"), david.essays + end + + def test_ids_on_unloaded_association_with_custom_primary_key + david = people(:david) + assert_equal Essay.where(writer_id: "David").pluck(:id), david.essay_ids + end + + def test_ids_on_loaded_association_with_custom_primary_key + david = people(:david) + david.essays.load + assert_equal Essay.where(writer_id: "David").pluck(:id), david.essay_ids end def test_has_many_assignment_with_custom_primary_key diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 4c2723addc..f8ea51225a 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -880,13 +880,6 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end end - def test_collection_singular_ids_setter_with_changed_primary_key - company = companies(:first_firm) - client = companies(:first_client) - company.clients_using_primary_key_ids = [client.name] - assert_equal [client], company.clients_using_primary_key - end - def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set company = companies(:rails_core) ids = [Developer.first.id, -9999] @@ -895,13 +888,6 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal(msg, e.message) end - def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set_with_changed_primary_key - company = companies(:first_firm) - ids = [Client.first.name, "unknown client"] - e = assert_raises(ActiveRecord::RecordNotFound) { company.clients_using_primary_key_ids = ids } - assert_match(/Couldn't find all Clients with 'name'/, e.message) - end - def test_collection_singular_ids_through_setter_raises_exception_when_invalid_ids_set author = authors(:david) ids = [categories(:general).name, "Unknown"] @@ -1139,6 +1125,32 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal ["parrot", "bulbul"], owner.toys.map { |r| r.pet.name } end + def test_has_many_through_associations_sum_on_columns + post1 = Post.create(title: "active", body: "sample") + post2 = Post.create(title: "inactive", body: "sample") + + person1 = Person.create(first_name: "aaron", followers_count: 1) + person2 = Person.create(first_name: "schmit", followers_count: 2) + person3 = Person.create(first_name: "bill", followers_count: 3) + person4 = Person.create(first_name: "cal", followers_count: 4) + + Reader.create(post_id: post1.id, person_id: person1.id) + Reader.create(post_id: post1.id, person_id: person2.id) + Reader.create(post_id: post1.id, person_id: person3.id) + Reader.create(post_id: post1.id, person_id: person4.id) + + Reader.create(post_id: post2.id, person_id: person1.id) + Reader.create(post_id: post2.id, person_id: person2.id) + Reader.create(post_id: post2.id, person_id: person3.id) + Reader.create(post_id: post2.id, person_id: person4.id) + + active_persons = Person.joins(:readers).joins(:posts).distinct(true).where("posts.title" => "active") + + assert_equal active_persons.map(&:followers_count).reduce(:+), 10 + assert_equal active_persons.sum(:followers_count), 10 + assert_equal active_persons.sum(:followers_count), active_persons.map(&:followers_count).reduce(:+) + end + def test_has_many_through_associations_on_new_records_use_null_relations person = Person.new @@ -1249,6 +1261,25 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal 0, users[2].family_members.to_a.size end + def test_through_scope_is_affected_by_unscoping + author = authors(:david) + + expected = author.comments.to_a + FirstPost.unscoped do + assert_equal expected.sort_by(&:id), author.comments_on_first_posts.sort_by(&:id) + end + end + + def test_through_scope_isnt_affected_by_scoping + author = authors(:david) + + expected = author.comments_on_first_posts.to_a + FirstPost.where(id: 2).scoping do + author.comments_on_first_posts.reset + assert_equal expected.sort_by(&:id), author.comments_on_first_posts.sort_by(&:id) + end + end + def test_incorrectly_ordered_through_associations assert_raises(ActiveRecord::HasManyThroughOrderError) do DeveloperWithIncorrectlyOrderedHasManyThrough.create( diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 9d3d5353ff..3abdcf3564 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -99,11 +99,11 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_polymorphic_has_many_create_model_with_inheritance_and_custom_base_class - post = SubStiPost.create title: "SubStiPost", body: "SubStiPost body" - assert_instance_of SubStiPost, post + post = SubAbstractStiPost.create title: "SubAbstractStiPost", body: "SubAbstractStiPost body" + assert_instance_of SubAbstractStiPost, post tagging = tags(:misc).taggings.create(taggable: post) - assert_equal "SubStiPost", tagging.taggable_type + assert_equal "SubAbstractStiPost", tagging.taggable_type end def test_polymorphic_has_many_going_through_join_model_with_inheritance diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 6b014bcb3d..b0b63f5203 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -132,7 +132,7 @@ class FixturesTest < ActiveRecord::TestCase def test_no_args_record_returns_all_without_array all_binaries = binaries assert_kind_of(Array, all_binaries) - assert_equal 1, binaries.length + assert_equal 2, binaries.length end def test_nil_raises @@ -313,6 +313,7 @@ class FixturesTest < ActiveRecord::TestCase data.force_encoding("ASCII-8BIT") data.freeze assert_equal data, @flowers.data + assert_equal data, @binary_helper.data end def test_serialized_fixtures diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index a263106f6d..c931f7d21c 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -147,12 +147,16 @@ class InheritanceTest < ActiveRecord::TestCase # Concrete subclass of AR::Base. assert Post.descends_from_active_record? + # Concrete subclasses of a concrete class which has a type column. + assert !StiPost.descends_from_active_record? + assert !SubStiPost.descends_from_active_record? + # Abstract subclass of a concrete class which has a type column. # This is pathological, as you'll never have Sub < Abstract < Concrete. - assert !StiPost.descends_from_active_record? + assert !AbstractStiPost.descends_from_active_record? - # Concrete subclasses an abstract class which has a type column. - assert !SubStiPost.descends_from_active_record? + # Concrete subclass of an abstract class which has a type column. + assert !SubAbstractStiPost.descends_from_active_record? end def test_company_descends_from_active_record @@ -172,7 +176,8 @@ class InheritanceTest < ActiveRecord::TestCase assert_equal Post, Post.base_class assert_equal Post, SpecialPost.base_class assert_equal Post, StiPost.base_class - assert_equal SubStiPost, SubStiPost.base_class + assert_equal Post, SubStiPost.base_class + assert_equal SubAbstractStiPost, SubAbstractStiPost.base_class end def test_abstract_inheritance_base_class diff --git a/activerecord/test/cases/json_shared_test_cases.rb b/activerecord/test/cases/json_shared_test_cases.rb index 952194c6dc..56ec8c8a82 100644 --- a/activerecord/test/cases/json_shared_test_cases.rb +++ b/activerecord/test/cases/json_shared_test_cases.rb @@ -216,7 +216,7 @@ module JSONSharedTestCases assert_equal true, json.payload end - def test_not_compatible_with_serialize_macro + def test_not_compatible_with_serialize_json new_klass = Class.new(klass) do serialize :payload, JSON end @@ -225,6 +225,30 @@ module JSONSharedTestCases end end + class MySettings + def initialize(hash); @hash = hash end + def to_hash; @hash end + def self.load(hash); new(hash) end + def self.dump(object); object.to_hash end + end + + def test_json_with_serialized_attributes + new_klass = Class.new(klass) do + serialize :settings, MySettings + end + + new_klass.create!(settings: MySettings.new("one" => "two")) + record = new_klass.first + + assert_instance_of MySettings, record.settings + assert_equal({ "one" => "two" }, record.settings.to_hash) + + record.settings = MySettings.new("three" => "four") + record.save! + + assert_equal({ "three" => "four" }, record.reload.settings.to_hash) + end + private def klass JsonDataType diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index fad55916c7..a10567f066 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -156,6 +156,13 @@ class UniquenessValidationTest < ActiveRecord::TestCase assert r3.valid?, "Saving r3" end + def test_validate_uniqueness_with_scope_invalid_syntax + error = assert_raises(ArgumentError) do + Reply.validates_uniqueness_of(:content, scope: { parent_id: false }) + end + assert_match(/Pass a symbol or an array of symbols instead/, error.to_s) + end + def test_validate_uniqueness_with_object_scope Reply.validates_uniqueness_of(:content, scope: :topic) diff --git a/activerecord/test/fixtures/binaries.yml b/activerecord/test/fixtures/binaries.yml index ec8f2facdc..53b7883369 100644 --- a/activerecord/test/fixtures/binaries.yml +++ b/activerecord/test/fixtures/binaries.yml @@ -131,3 +131,7 @@ flowers: SgCUASgCUASgCUASgAC74PbXOTvE5/En7jpSoLE8/wBn7uPJjKyj46T9D/NT pKsXyQzxNpdNP0/akB5484WkMKh4RfXG4UafNmH7b0UxWMrb7Nxg6rl9Z/Im w+vWq0iscQwxQroiUIvkKsRZQBKAJQBKAJQB/9k= + +binary_helper: + id: 2 + data: <%= binary(ASSETS_ROOT + "/flowers.jpg") %> diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 4b3576fce8..4c8e847354 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -184,14 +184,19 @@ end class SpecialPost < Post; end class StiPost < Post - self.abstract_class = true has_one :special_comment, class_name: "SpecialComment" end +class AbstractStiPost < Post + self.abstract_class = true +end + class SubStiPost < StiPost self.table_name = Post.table_name end +class SubAbstractStiPost < AbstractStiPost; end + class FirstPost < ActiveRecord::Base self.inheritance_column = :disabled self.table_name = "posts" diff --git a/activestorage/README.md b/activestorage/README.md index d4ffe0c484..17d98978bb 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -1,17 +1,16 @@ # Active Storage -Active Storage makes it simple to upload and reference files in cloud services, like Amazon S3, Google Cloud Storage or Microsoft Azure Storage and attach those files to Active Records. It also provides a disk service for testing or local deployments, but the focus is on cloud storage. +Active Storage makes it simple to upload and reference files in cloud services like Amazon S3, Google Cloud Storage, or Microsoft Azure Storage, and attach those files to Active Records. Supports having one main service and mirrors in other services for redundancy. It also provides a disk service for testing or local deployments, but the focus is on cloud storage. Files can be uploaded from the server to the cloud or directly from the client to the cloud. -Image files can further more be transformed using on-demand variants for quality, aspect ratio, size, or any other -MiniMagick supported transformation. +Image files can furthermore be transformed using on-demand variants for quality, aspect ratio, size, or any other [MiniMagick](https://github.com/minimagick/minimagick) supported transformation. ## Compared to other storage solutions -A key difference to how Active Storage works compared to other attachment solutions in Rails is through the use of built-in [Blob](https://github.com/rails/rails/blob/master/activestorage/app/models/active_storage/blob.rb) and [Attachment](https://github.com/rails/rails/blob/master/activestorage/app/models/active_storage/attachment.rb) models (backed by Active Record). This means existing application models do not need to be modified with additional columns to associate with files. Active Storage uses polymorphic associations via the join model of `Attachment`, which then connects to the actual `Blob`. +A key difference to how Active Storage works compared to other attachment solutions in Rails is through the use of built-in [Blob](https://github.com/rails/rails/blob/master/activestorage/app/models/active_storage/blob.rb) and [Attachment](https://github.com/rails/rails/blob/master/activestorage/app/models/active_storage/attachment.rb) models (backed by Active Record). This means existing application models do not need to be modified with additional columns to associate with files. Active Storage uses polymorphic associations via the `Attachment` join model, which then connects to the actual `Blob`. -These `Blob` models are intended to be immutable in spirit. One file, one blob. You can associate the same blob with multiple application models as well. And if you want to do transformations of a given `Blob`, the idea is that you'll simply create a new one, rather than attempt to mutate the existing (though of course you can delete that later if you don't need it). +`Blob` models store attachment metadata (filename, content-type, etc.), and their identifier key in the storage service. Blob models do not store the actual binary data. They are intended to be immutable in spirit. One file, one blob. You can associate the same blob with multiple application models as well. And if you want to do transformations of a given `Blob`, the idea is that you'll simply create a new one, rather than attempt to mutate the existing one (though of course you can delete the previous version later if you don't need it). ## Examples @@ -19,16 +18,32 @@ One attachment: ```ruby class User < ApplicationRecord + # Associates an attachment and a blob. When the user is destroyed they are + # purged by default (models destroyed, and resource files deleted). has_one_attached :avatar end -user.avatar.attach io: File.open("~/face.jpg"), filename: "avatar.jpg", content_type: "image/jpg" +# Attach an avatar to the user. +user.avatar.attach(io: File.open("~/face.jpg"), filename: "avatar.jpg", content_type: "image/jpg") + +# Does the user have an avatar? user.avatar.attached? # => true +# Synchronously destroy the avatar and actual resource files. user.avatar.purge + +# Destroy the associated models and actual resource files async, via Active Job. +user.avatar.purge_later + +# Does the user have an avatar? user.avatar.attached? # => false -url_for(user.avatar) # Generate a permanent URL for the blob, which upon access will redirect to a temporary service URL. +# Generate a permanent URL for the blob that points to the application. +# Upon access, a redirect to the actual service endpoint is returned. +# This indirection decouples the public URL from the actual one, and +# allows for example mirroring attachments in different services for +# high-availability. The redirection has an HTTP expiration of 5 min. +url_for(user.avatar) class AvatarsController < ApplicationController def update diff --git a/activestorage/app/jobs/active_storage/purge_job.rb b/activestorage/app/jobs/active_storage/purge_job.rb index 404ccefd05..369c07929d 100644 --- a/activestorage/app/jobs/active_storage/purge_job.rb +++ b/activestorage/app/jobs/active_storage/purge_job.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Provides delayed purging of attachments or blobs using their +#purge_later+ method. +# Provides delayed purging of attachments or blobs using their +purge_later+ method. class ActiveStorage::PurgeJob < ActiveJob::Base # FIXME: Limit this to a custom ActiveStorage error retry_on StandardError diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb index adfa6d4aa5..ad43845e4e 100644 --- a/activestorage/app/models/active_storage/attachment.rb +++ b/activestorage/app/models/active_storage/attachment.rb @@ -4,7 +4,7 @@ require "active_support/core_ext/module/delegation" # Attachments associate records with blobs. Usually that's a one record-many blobs relationship, # but it is possible to associate many different records with the same blob. If you're doing that, -# you'll want to declare with `has_one/many_attached :thingy, dependent: false`, so that destroying +# you'll want to declare with <tt>has_one/many_attached :thingy, dependent: false</tt>, so that destroying # any one record won't destroy the blob as well. (Then you'll need to do your own garbage collecting, though). class ActiveStorage::Attachment < ActiveRecord::Base self.table_name = "active_storage_attachments" @@ -22,8 +22,8 @@ class ActiveStorage::Attachment < ActiveRecord::Base end # Purging an attachment means purging the blob, which means talking to the service, which means - # talking over the internet. Whenever you're doing that, it's a good idea to put that work in a job, - # so it doesn't hold up other operations. That's what +#purge_later+ provides. + # talking over the Internet. Whenever you're doing that, it's a good idea to put that work in a job, + # so it doesn't hold up other operations. That's what +purge_later+ provides. def purge_later ActiveStorage::PurgeJob.perform_later(self) end diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index dc91a9265f..9f2ed1e5ac 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -3,8 +3,8 @@ # A blob is a record that contains the metadata about a file and a key for where that file resides on the service. # Blobs can be created in two ways: # -# 1) Subsequent to the file being uploaded server-side to the service via #create_after_upload! -# 2) Ahead of the file being directly uploaded client-side to the service via #create_before_direct_upload! +# 1) Subsequent to the file being uploaded server-side to the service via <tt>create_after_upload!</tt>. +# 2) Ahead of the file being directly uploaded client-side to the service via <tt>create_before_direct_upload!</tt>. # # The first option doesn't require any client-side JavaScript integration, and can be used by any other back-end # service that deals with files. The second option is faster, since you're not using your own server as a staging @@ -12,7 +12,7 @@ # # Blobs are intended to be immutable in as-so-far as their reference to a specific file goes. You're allowed to # update a blob's metadata on a subsequent pass, but you should not update the key or change the uploaded file. -# If you need to create a derivative or otherwise change the blob, simply create a new blob and purge the old. +# If you need to create a derivative or otherwise change the blob, simply create a new blob and purge the old one. class ActiveStorage::Blob < ActiveRecord::Base self.table_name = "active_storage_blobs" @@ -22,11 +22,11 @@ class ActiveStorage::Blob < ActiveRecord::Base class_attribute :service class << self - # You can used the signed id of a blob to refer to it on the client side without fear of tampering. - # This is particularly helpful for direct uploads where the client side needs to refer to the blob + # You can used the signed ID of a blob to refer to it on the client side without fear of tampering. + # This is particularly helpful for direct uploads where the client-side needs to refer to the blob # that was created ahead of the upload itself on form submission. # - # The signed id is also used to create stable URLs for the blob through the BlobsController. + # The signed ID is also used to create stable URLs for the blob through the BlobsController. def find_signed(id) find ActiveStorage.verifier.verify(id, purpose: :blob_id) end @@ -43,8 +43,8 @@ class ActiveStorage::Blob < ActiveRecord::Base end # Returns a saved blob instance after the +io+ has been uploaded to the service. Note, the blob is first built, - # then the +io+ is uploaded, then the blob is saved. This is doing to avoid opening a transaction and talking to - # the service during that (which is a bad idea and leads to deadlocks). + # then the +io+ is uploaded, then the blob is saved. This is done this way to avoid uploading (which may take + # time), while having an open database transaction. def create_after_upload!(io:, filename:, content_type: nil, metadata: nil) build_after_upload(io: io, filename: filename, content_type: content_type, metadata: metadata).tap(&:save!) end @@ -61,7 +61,7 @@ class ActiveStorage::Blob < ActiveRecord::Base # Returns a signed ID for this blob that's suitable for reference on the client-side without fear of tampering. - # It uses the framework-wide verifier on `ActiveStorage.verifier`, but with a dedicated purpose. + # It uses the framework-wide verifier on <tt>ActiveStorage.verifier</tt>, but with a dedicated purpose. def signed_id ActiveStorage.verifier.generate(id, purpose: :blob_id) end @@ -74,8 +74,9 @@ class ActiveStorage::Blob < ActiveRecord::Base self[:key] ||= self.class.generate_unique_secure_token end - # Returns a ActiveStorage::Filename instance of the filename that can be queried for basename, extension, and - # a sanitized version of the filename that's safe to use in URLs. + # Returns an ActiveStorage::Filename instance of the filename that can be + # queried for basename, extension, and a sanitized version of the filename + # that's safe to use in URLs. def filename ActiveStorage::Filename.new(self[:filename]) end @@ -100,8 +101,9 @@ class ActiveStorage::Blob < ActiveRecord::Base content_type.start_with?("text") end - # Returns a ActiveStorage::Variant instance with the set of +transformations+ passed in. This is only relevant - # for image files, and it allows any image to be transformed for size, colors, and the like. Example: + # Returns an ActiveStorage::Variant instance with the set of +transformations+ + # passed in. This is only relevant for image files, and it allows any image to + # be transformed for size, colors, and the like. Example: # # avatar.variant(resize: "100x100").processed.service_url # @@ -178,7 +180,7 @@ class ActiveStorage::Blob < ActiveRecord::Base destroy end - # Enqueues a ActiveStorage::PurgeJob job that'll call +#purge+. This is the recommended way to purge blobs when the call + # Enqueues an ActiveStorage::PurgeJob job that'll call +purge+. This is the recommended way to purge blobs when the call # needs to be made from a transaction, a callback, or any other real-time scenario. def purge_later ActiveStorage::PurgeJob.perform_later(self) diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index 7b4ca18c8c..40648a27f7 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -4,8 +4,8 @@ # These variants are used to create thumbnails, fixed-size avatars, or any other derivative image from the # original. # -# Variants rely on `MiniMagick` for the actual transformations of the file, so you must add `gem "mini_magick"` -# to your Gemfile if you wish to use variants. +# Variants rely on {MiniMagick}(https://github.com/minimagick/minimagick) for the actual transformations +# of the file, so you must add <tt>gem "mini_magick"</tt> to your Gemfile if you wish to use variants. # # Note that to create a variant it's necessary to download the entire blob file from the service and load it # into memory. The larger the image, the more memory is used. Because of this process, you also want to be @@ -21,7 +21,7 @@ # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::VariantsController # can then produce on-demand. # -# When you do want to actually produce the variant needed, call +#processed+. This will check that the variant +# When you do want to actually produce the variant needed, call +processed+. This will check that the variant # has already been processed and uploaded to the service, and, if so, just return that. Otherwise it will perform # the transformations, upload the variant to the service, and return itself again. Example: # @@ -58,8 +58,8 @@ class ActiveStorage::Variant # Hiding the +service_url+ behind a redirect also gives you the power to change services without updating all URLs. And # it allows permanent URLs that redirect to the +service_url+ to be cached in the view. # - # Use `url_for(variant)` (or the implied form, like `link_to variant` or `redirect_to variant`) to get the stable URL - # for a variant that points to the ActiveStorage::VariantsController, which in turn will use this +#service_call+ method + # Use <tt>url_for(variant)</tt> (or the implied form, like +link_to variant+ or +redirect_to variant+) to get the stable URL + # 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: 5.minutes, disposition: :inline) service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename, content_type: blob.content_type diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index 396ce85ef1..f657d90db4 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -15,13 +15,13 @@ class ActiveStorage::Variation attr_reader :transformations class << self - # Returns a variation instance with the transformations that were encoded by +#encode+. + # Returns a variation instance with the transformations that were encoded by +encode+. def decode(key) new ActiveStorage.verifier.verify(key, purpose: :variation) end # Returns a signed key for the +transformations+, which can be used to refer to a specific - # variation in a URL or combined key (like `ActiveStorage::Variant#key`). + # variation in a URL or combined key (like <tt>ActiveStorage::Variant#key</tt>). def encode(transformations) ActiveStorage.verifier.generate(transformations, purpose: :variation) end @@ -31,7 +31,7 @@ class ActiveStorage::Variation @transformations = transformations end - # Accepts an open MiniMagick image instance, like what's return by `MiniMagick::Image.read(io)`, + # Accepts an open MiniMagick image instance, like what's returned by <tt>MiniMagick::Image.read(io)</tt>, # and performs the +transformations+ against it. The transformed image instance is then returned. def transform(image) transformations.each do |(method, argument)| diff --git a/activestorage/lib/active_storage/attached.rb b/activestorage/lib/active_storage/attached.rb index e90b75afd0..9c4b6d5d1d 100644 --- a/activestorage/lib/active_storage/attached.rb +++ b/activestorage/lib/active_storage/attached.rb @@ -5,8 +5,8 @@ require "action_dispatch/http/upload" require "active_support/core_ext/module/delegation" module ActiveStorage -# Abstract baseclass for the concrete ActiveStorage::Attached::One and ActiveStorage::Attached::Many -# classes that both provide proxy access to the blob association for a record. + # Abstract base class for the concrete ActiveStorage::Attached::One and ActiveStorage::Attached::Many + # classes that both provide proxy access to the blob association for a record. class Attached attr_reader :name, :record diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb index 027112195f..f3879ee2e3 100644 --- a/activestorage/lib/active_storage/attached/macros.rb +++ b/activestorage/lib/active_storage/attached/macros.rb @@ -19,7 +19,7 @@ module ActiveStorage # most circumstances. # # The system has been designed to having you go through the ActiveStorage::Attached::One - # proxy that provides the dynamic proxy to the associations and factory methods, like +#attach+. + # proxy that provides the dynamic proxy to the associations and factory methods, like +attach+. # # If the +:dependent+ option isn't set, the attachment will be purged # (i.e. destroyed) whenever the record is destroyed. diff --git a/activestorage/lib/active_storage/gem_version.rb b/activestorage/lib/active_storage/gem_version.rb index bb6241ae47..e1d7b3493a 100644 --- a/activestorage/lib/active_storage/gem_version.rb +++ b/activestorage/lib/active_storage/gem_version.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module ActiveStorage - # Returns the version of the currently loaded Active Storage as a <tt>Gem::Version</tt> + # Returns the version of the currently loaded Active Storage as a <tt>Gem::Version</tt>. def self.gem_version Gem::Version.new VERSION::STRING end diff --git a/activestorage/lib/active_storage/log_subscriber.rb b/activestorage/lib/active_storage/log_subscriber.rb index 80cc5d5c80..0d00a75c0e 100644 --- a/activestorage/lib/active_storage/log_subscriber.rb +++ b/activestorage/lib/active_storage/log_subscriber.rb @@ -19,7 +19,7 @@ module ActiveStorage end def service_exist(event) - debug event, color("Checked if file exist at key: #{key_in(event)} (#{event.payload[:exist] ? "yes" : "no"})", BLUE) + debug event, color("Checked if file exists at key: #{key_in(event)} (#{event.payload[:exist] ? "yes" : "no"})", BLUE) end def service_url(event) diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index d066deaa20..d0ee2273c5 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -5,7 +5,7 @@ require "azure/storage" require "azure/storage/core/auth/shared_access_signature" module ActiveStorage - # Wraps the Microsoft Azure Storage Blob Service as a Active Storage service. + # Wraps the Microsoft Azure Storage Blob Service as an Active Storage service. # See ActiveStorage::Service for the generic API documentation that applies to all services. class Service::AzureStorageService < Service attr_reader :client, :path, :blobs, :container, :signer diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index cf4019966f..0e38f88138 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -6,7 +6,7 @@ require "digest/md5" require "active_support/core_ext/numeric/bytes" module ActiveStorage - # Wraps a local disk path as a Active Storage service. See ActiveStorage::Service for the generic API + # Wraps a local disk path as an Active Storage service. See ActiveStorage::Service for the generic API # documentation that applies to all services. class Service::DiskService < Service attr_reader :root diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index 32dfb4851b..e656f2c69f 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -4,7 +4,7 @@ require "google/cloud/storage" require "active_support/core_ext/object/to_query" module ActiveStorage - # Wraps the Google Cloud Storage as a Active Storage service. See ActiveStorage::Service for the generic API + # Wraps the Google Cloud Storage as an Active Storage service. See ActiveStorage::Service for the generic API # documentation that applies to all services. class Service::GCSService < Service attr_reader :client, :bucket diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 98c9dd2972..bef2ecbea9 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -4,7 +4,7 @@ require "aws-sdk" require "active_support/core_ext/numeric/bytes" module ActiveStorage - # Wraps the Amazon Simple Storage Service (S3) as a Active Storage service. + # Wraps the Amazon Simple Storage Service (S3) as an Active Storage service. # See ActiveStorage::Service for the generic API documentation that applies to all services. class Service::S3Service < Service attr_reader :client, :bucket, :upload_options diff --git a/guides/source/command_line.md b/guides/source/command_line.md index 9fddbf76b6..8ae01286e4 100644 --- a/guides/source/command_line.md +++ b/guides/source/command_line.md @@ -102,6 +102,7 @@ Please choose a generator below. Rails: assets + channel controller generator ... @@ -241,6 +242,8 @@ $ bin/rails generate scaffold HighScore game:string score:integer invoke jbuilder create app/views/high_scores/index.json.jbuilder create app/views/high_scores/show.json.jbuilder + invoke test_unit + create test/system/high_scores_test.rb invoke assets invoke coffee create app/assets/javascripts/high_scores.coffee diff --git a/guides/source/engines.md b/guides/source/engines.md index 2276f348a1..c7331b6ca4 100644 --- a/guides/source/engines.md +++ b/guides/source/engines.md @@ -346,6 +346,9 @@ invoke test_unit create test/controllers/blorgh/articles_controller_test.rb invoke helper create app/helpers/blorgh/articles_helper.rb +invoke test_unit +create test/application_system_test_case.rb +create test/system/articles_test.rb invoke assets invoke js create app/assets/javascripts/blorgh/articles.js diff --git a/guides/source/generators.md b/guides/source/generators.md index 389224d908..37af7a1310 100644 --- a/guides/source/generators.md +++ b/guides/source/generators.md @@ -197,6 +197,9 @@ $ bin/rails generate scaffold User name:string invoke jbuilder create app/views/users/index.json.jbuilder create app/views/users/show.json.jbuilder + invoke test_unit + create test/application_system_test_case.rb + create test/system/users_test.rb invoke assets invoke coffee create app/assets/javascripts/users.coffee @@ -415,6 +418,9 @@ $ bin/rails generate scaffold Comment body:text invoke jbuilder create app/views/comments/index.json.jbuilder create app/views/comments/show.json.jbuilder + invoke test_unit + create test/application_system_test_case.rb + create test/system/comments_test.rb invoke assets invoke coffee create app/assets/javascripts/comments.coffee diff --git a/guides/source/plugins.md b/guides/source/plugins.md index 8c2d56ceb8..164207a9f9 100644 --- a/guides/source/plugins.md +++ b/guides/source/plugins.md @@ -74,7 +74,7 @@ In this example you will add a method to String named `to_squawk`. To begin, cre ```ruby # yaffle/test/core_ext_test.rb -require 'test_helper' +require "test_helper" class CoreExtTest < ActiveSupport::TestCase def test_to_squawk_prepends_the_word_squawk @@ -104,14 +104,16 @@ Finished in 0.003358s, 595.6483 runs/s, 297.8242 assertions/s. Great - now you are ready to start development. -In `lib/yaffle.rb`, add `require 'yaffle/core_ext'`: +In `lib/yaffle.rb`, add `require "yaffle/core_ext"`: ```ruby # yaffle/lib/yaffle.rb -require 'yaffle/core_ext' +require "yaffle/railtie" +require "yaffle/core_ext" module Yaffle + # Your code goes here... end ``` @@ -120,7 +122,7 @@ Finally, create the `core_ext.rb` file and add the `to_squawk` method: ```ruby # yaffle/lib/yaffle/core_ext.rb -String.class_eval do +class String def to_squawk "squawk! #{self}".strip end @@ -152,7 +154,7 @@ To begin, set up your files so that you have: ```ruby # yaffle/test/acts_as_yaffle_test.rb -require 'test_helper' +require "test_helper" class ActsAsYaffleTest < ActiveSupport::TestCase end @@ -161,10 +163,12 @@ end ```ruby # yaffle/lib/yaffle.rb -require 'yaffle/core_ext' -require 'yaffle/acts_as_yaffle' +require "yaffle/railtie" +require "yaffle/core_ext" +require "yaffle/acts_as_yaffle" module Yaffle + # Your code goes here... end ``` @@ -173,7 +177,6 @@ end module Yaffle module ActsAsYaffle - # your code will go here end end ``` @@ -189,7 +192,7 @@ To start out, write a failing test that shows the behavior you'd like: ```ruby # yaffle/test/acts_as_yaffle_test.rb -require 'test_helper' +require "test_helper" class ActsAsYaffleTest < ActiveSupport::TestCase def test_a_hickwalls_yaffle_text_field_should_be_last_squawk @@ -276,12 +279,8 @@ module Yaffle module ActsAsYaffle extend ActiveSupport::Concern - included do - end - - module ClassMethods + class_methods do def acts_as_yaffle(options = {}) - # your code will go here end end end @@ -335,10 +334,7 @@ module Yaffle module ActsAsYaffle extend ActiveSupport::Concern - included do - end - - module ClassMethods + class_methods do def acts_as_yaffle(options = {}) cattr_accessor :yaffle_text_field, default: (options[:yaffle_text_field] || :last_squawk).to_s end @@ -370,7 +366,7 @@ To start out, write a failing test that shows the behavior you'd like: ```ruby # yaffle/test/acts_as_yaffle_test.rb -require 'test_helper' +require "test_helper" class ActsAsYaffleTest < ActiveSupport::TestCase def test_a_hickwalls_yaffle_text_field_should_be_last_squawk @@ -406,19 +402,14 @@ module Yaffle extend ActiveSupport::Concern included do + def squawk(string) + write_attribute(self.class.yaffle_text_field, string.to_squawk) + end end - module ClassMethods + class_methods do def acts_as_yaffle(options = {}) cattr_accessor :yaffle_text_field, default: (options[:yaffle_text_field] || :last_squawk).to_s - - include Yaffle::ActsAsYaffle::LocalInstanceMethods - end - end - - module LocalInstanceMethods - def squawk(string) - write_attribute(self.class.yaffle_text_field, string.to_squawk) end end end @@ -449,7 +440,7 @@ Generators ---------- Generators can be included in your gem simply by creating them in a lib/generators directory of your plugin. More information about -the creation of generators can be found in the [Generators Guide](generators.html) +the creation of generators can be found in the [Generators Guide](generators.html). Publishing Your Gem ------------------- @@ -458,13 +449,13 @@ Gem plugins currently in development can easily be shared from any Git repositor commit the code to a Git repository (like GitHub) and add a line to the Gemfile of the application in question: ```ruby -gem 'yaffle', git: 'git://github.com/yaffle_watcher/yaffle.git' +gem "yaffle", git: "https://github.com/rails/yaffle.git" ``` After running `bundle install`, your gem functionality will be available to the application. -When the gem is ready to be shared as a formal release, it can be published to [RubyGems](http://www.rubygems.org). -For more information about publishing gems to RubyGems, see: [Creating and Publishing Your First Ruby Gem](http://blog.thepete.net/2010/11/creating-and-publishing-your-first-ruby.html). +When the gem is ready to be shared as a formal release, it can be published to [RubyGems](https://rubygems.org). +For more information about publishing gems to RubyGems, see: [Publishing your gem](http://guides.rubygems.org/publishing). RDoc Documentation ------------------ @@ -478,7 +469,7 @@ The first step is to update the README file with detailed information about how * How to add the functionality to the app (several examples of common use cases) * Warnings, gotchas or tips that might help users and save them time -Once your README is solid, go through and add rdoc comments to all of the methods that developers will use. It's also customary to add '#:nodoc:' comments to those parts of the code that are not included in the public API. +Once your README is solid, go through and add rdoc comments to all of the methods that developers will use. It's also customary to add `#:nodoc:` comments to those parts of the code that are not included in the public API. Once your comments are good to go, navigate to your plugin directory and run: diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 17d9ff8116..7b720d6e18 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add `--skip-yarn` option to the plugin generator. + + *bogdanvlviv* + * Optimize routes indentation. *Yoshiyuki Hirano* @@ -10,7 +14,7 @@ *Yoshiyuki Hirano* -* Add git_source to `Gemfile` for plugin generator. +* Add `git_source` to `Gemfile` for plugin generator. *Yoshiyuki Hirano* @@ -80,7 +84,7 @@ * Load environment file in `dbconsole` command. - Fixes #29717 + Fixes #29717. *Yuji Yaginuma* diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index eda12b6da2..c773e07eba 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -145,7 +145,7 @@ module Rails create_file("vendor/#{filename}", optimize_indentation(data), verbose: false) end - # Create a new file in the lib/ directory. Code can be specified + # Create a new file in the <tt>lib/</tt> directory. Code can be specified # in a block or a data string can be given. # # lib("crypto.rb") do diff --git a/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb.tt b/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb.tt index 185c0017f1..938eff8ed0 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb.tt @@ -1,2 +1,2 @@ -class ApplicationController < ActionController::<%= options[:api] ? "API" : "Base" %> +class ApplicationController < ActionController::<%= options.api? ? "API" : "Base" %> end diff --git a/railties/lib/rails/generators/rails/app/templates/bin/setup.tt b/railties/lib/rails/generators/rails/app/templates/bin/setup.tt index a53d1fb0a9..233b5a1d95 100644 --- a/railties/lib/rails/generators/rails/app/templates/bin/setup.tt +++ b/railties/lib/rails/generators/rails/app/templates/bin/setup.tt @@ -15,12 +15,12 @@ chdir APP_ROOT do puts '== Installing dependencies ==' system! 'gem install bundler --conservative' system('bundle check') || system!('bundle install') -<% unless options[:skip_yarn] -%> +<% unless options.skip_yarn? -%> # Install JavaScript dependencies if using Yarn # system('bin/yarn') <% end -%> -<% unless options.skip_active_record -%> +<% unless options.skip_active_record? -%> # puts "\n== Copying sample files ==" # unless File.exist?('config/database.yml') diff --git a/railties/lib/rails/generators/rails/app/templates/bin/update.tt b/railties/lib/rails/generators/rails/app/templates/bin/update.tt index 5b6e50883e..d744bec32f 100644 --- a/railties/lib/rails/generators/rails/app/templates/bin/update.tt +++ b/railties/lib/rails/generators/rails/app/templates/bin/update.tt @@ -15,7 +15,7 @@ chdir APP_ROOT do puts '== Installing dependencies ==' system! 'gem install bundler --conservative' system('bundle check') || system!('bundle install') -<% unless options.skip_active_record -%> +<% unless options.skip_active_record? -%> puts "\n== Updating database ==" system! 'bin/rails db:migrate' diff --git a/railties/lib/rails/generators/rails/app/templates/config/application.rb b/railties/lib/rails/generators/rails/app/templates/config/application.rb index e9edad5309..eec973caa5 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/application.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/application.rb @@ -31,7 +31,7 @@ module <%= app_const_base %> # Settings in config/environments/* take precedence over those specified here. # Application configuration should go into files in config/initializers # -- all .rb files in that directory are automatically loaded. -<%- if options[:api] -%> +<%- if options.api? -%> # Only loads a smaller set of middleware suitable for API only apps. # Middleware like session, flash, cookies can be added back manually. diff --git a/railties/lib/rails/generators/rails/app/templates/gitignore b/railties/lib/rails/generators/rails/app/templates/gitignore index 64ed09dc4b..83a7b211aa 100644 --- a/railties/lib/rails/generators/rails/app/templates/gitignore +++ b/railties/lib/rails/generators/rails/app/templates/gitignore @@ -24,13 +24,12 @@ # Ignore uploaded files in development /storage/* -<% unless options[:skip_yarn] -%> +<% unless options.skip_yarn? -%> /node_modules /yarn-error.log - <% end -%> -<% unless options[:api] -%> +<% unless options.api? -%> /public/assets <% end -%> .byebug_history diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index a0de708913..eb941adf95 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -89,7 +89,7 @@ task default: :test PASSTHROUGH_OPTIONS = [ :skip_active_record, :skip_action_mailer, :skip_javascript, :skip_action_cable, :skip_sprockets, :database, - :javascript, :api, :quiet, :pretend, :skip + :javascript, :skip_yarn, :api, :quiet, :pretend, :skip ] def generate_test_dummy(force = false) diff --git a/railties/lib/rails/generators/rails/plugin/templates/gitignore b/railties/lib/rails/generators/rails/plugin/templates/gitignore index 757172e6a6..8c7cad74ed 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/gitignore +++ b/railties/lib/rails/generators/rails/plugin/templates/gitignore @@ -5,5 +5,9 @@ pkg/ <%= dummy_path %>/db/*.sqlite3 <%= dummy_path %>/db/*.sqlite3-journal <%= dummy_path %>/log/*.log +<% unless options[:skip_yarn] -%> +<%= dummy_path %>/node_modules/ +<%= dummy_path %>/yarn-error.log +<% end -%> <%= dummy_path %>/tmp/ <% end -%> diff --git a/railties/test/generators/api_app_generator_test.rb b/railties/test/generators/api_app_generator_test.rb index 58617510d7..035734ca60 100644 --- a/railties/test/generators/api_app_generator_test.rb +++ b/railties/test/generators/api_app_generator_test.rb @@ -35,26 +35,26 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase def test_api_modified_files run_generator + assert_file ".gitignore" do |content| + assert_no_match(/\/public\/assets/, content) + end + assert_file "Gemfile" do |content| assert_no_match(/gem 'coffee-rails'/, content) assert_no_match(/gem 'sass-rails'/, content) assert_no_match(/gem 'web-console'/, content) + assert_no_match(/gem 'capybara'/, content) + assert_no_match(/gem 'selenium-webdriver'/, content) assert_match(/# gem 'jbuilder'/, content) + assert_match(/# gem 'rack-cors'/, content) end - assert_file "config/application.rb" do |content| - assert_match(/config.api_only = true/, content) - end - - assert_file "config/initializers/cors.rb" - - assert_file "config/initializers/wrap_parameters.rb" - + assert_file "config/application.rb", /config\.api_only = true/ assert_file "app/controllers/application_controller.rb", /ActionController::API/ end def test_generator_if_skip_action_cable_is_given - run_generator [destination_root, "--skip-action-cable"] + run_generator [destination_root, "--api", "--skip-action-cable"] assert_file "config/application.rb", /#\s+require\s+["']action_cable\/engine["']/ assert_no_file "config/cable.yml" assert_no_file "app/channels" @@ -87,20 +87,49 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase private def default_files - files = %W( - .gitignore + %w(.gitignore + .ruby-version + README.md Gemfile Rakefile config.ru + app/channels app/controllers app/mailers app/models + app/views/layouts app/views/layouts/mailer.html.erb app/views/layouts/mailer.text.erb + bin/bundle + bin/rails + bin/rake + bin/setup + bin/update + config/application.rb + config/boot.rb + config/cable.yml + config/environment.rb config/environments + config/environments/development.rb + config/environments/production.rb + config/environments/test.rb config/initializers + config/initializers/application_controller_renderer.rb + config/initializers/backtrace_silencers.rb + config/initializers/cors.rb + config/initializers/filter_parameter_logging.rb + config/initializers/inflections.rb + config/initializers/mime_types.rb + config/initializers/wrap_parameters.rb config/locales + config/locales/en.yml + config/puma.rb + config/routes.rb + config/secrets.yml + config/spring.rb + config/storage.yml db + db/seeds.rb lib lib/tasks log @@ -111,8 +140,6 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase tmp vendor ) - files.concat %w(bin/bundle bin/rails bin/rake) - files end def skipped_files @@ -130,7 +157,7 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase public/500.html public/apple-touch-icon-precomposed.png public/apple-touch-icon.png - public/favicon.icon + public/favicon.ico package.json ) end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index ccb437c74d..94a685393b 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -537,14 +537,6 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def test_generator_if_api_is_given - run_generator [destination_root, "--api"] - assert_file "Gemfile" do |content| - assert_no_match(/capybara/, content) - assert_no_match(/selenium-webdriver/, content) - end - end - def test_inclusion_of_javascript_runtime run_generator if defined?(JRUBY_VERSION) @@ -592,32 +584,6 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def test_generator_for_yarn - 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 - run_generator([destination_root, "--skip-yarn"]) - assert_no_file "package.json" - assert_no_file "bin/yarn" - - assert_file "config/initializers/assets.rb" do |content| - assert_no_match(/node_modules/, content) - end - - assert_file ".gitignore" do |content| - assert_no_match(/node_modules/, content) - assert_no_match(/yarn-error\.log/, content) - end - end - def test_inclusion_of_jbuilder run_generator assert_gem "jbuilder" diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index bd76af86e1..e94b1ac8fe 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -26,6 +26,10 @@ class PluginGeneratorTest < Rails::Generators::TestCase destination File.join(Rails.root, "tmp/bukkits") arguments [destination_root] + def application_path + "#{destination_root}/test/dummy" + end + # brings setup, teardown, and some tests include SharedGeneratorTests diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index c73b91e3f8..6c0775b50e 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -22,6 +22,10 @@ module SharedGeneratorTests Rails.application = TestApp::Application.instance end + def application_path + destination_root + end + def test_skeleton_is_created run_generator @@ -123,4 +127,30 @@ module SharedGeneratorTests assert_no_file("app/models/concerns/.keep") end + + def test_generator_for_yarn + run_generator + assert_file "#{application_path}/package.json", /dependencies/ + assert_file "#{application_path}/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 + run_generator([destination_root, "--skip-yarn"]) + assert_no_file "#{application_path}/package.json" + assert_no_file "#{application_path}/bin/yarn" + + assert_file "#{application_path}/config/initializers/assets.rb" do |content| + assert_no_match(/node_modules/, content) + end + + assert_file ".gitignore" do |content| + assert_no_match(/node_modules/, content) + assert_no_match(/yarn-error\.log/, content) + end + end end |