diff options
78 files changed, 1314 insertions, 270 deletions
diff --git a/.travis.yml b/.travis.yml index f3314f49af..daaa530faa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,17 +1,33 @@ language: ruby sudo: false -script: 'ci/travis.rb' + +cache: + bundler: true + directories: + - /tmp/cache/unicode_conformance + - /tmp/beanstalkd-1.10 + +services: + - memcached + - redis + - rabbitmq + +addons: + postgresql: "9.4" + +bundler_args: --without test --jobs 3 --retry 3 + before_install: - gem install bundler - "rm ${BUNDLE_GEMFILE}.lock" - - curl -L https://github.com/kr/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp - - cd /tmp/beanstalkd-1.10/ - - make - - ./beanstalkd & - - cd $TRAVIS_BUILD_DIR + - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/kr/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" + - "pushd /tmp/beanstalkd-1.10 && make && (./beanstalkd &); popd" + before_script: - bundle update -cache: bundler + +script: 'ci/travis.rb' + env: matrix: - "GEM=railties" @@ -24,10 +40,12 @@ env: - "GEM=ar:postgresql" - "GEM=aj:integration" - "GEM=guides" + rvm: - 2.2.4 - 2.3.0 - ruby-head + matrix: include: # Latest compiled version in http://rubies.travis-ci.org @@ -44,6 +62,7 @@ matrix: allow_failures: - rvm: ruby-head fast_finish: true + notifications: email: false irc: @@ -56,10 +75,3 @@ notifications: on_failure: always rooms: - secure: "YA1alef1ESHWGFNVwvmVGCkMe4cUy4j+UcNvMUESraceiAfVyRMAovlQBGs6\n9kBRm7DHYBUXYC2ABQoJbQRLDr/1B5JPf/M8+Qd7BKu8tcDC03U01SMHFLpO\naOs/HLXcDxtnnpL07tGVsm0zhMc5N8tq4/L3SHxK7Vi+TacwQzI=" -bundler_args: --without test --jobs 3 --retry 3 -services: - - memcached - - redis - - rabbitmq -addons: - postgresql: "9.4" diff --git a/actioncable/README.md b/actioncable/README.md index de67b507a7..fe4d213485 100644 --- a/actioncable/README.md +++ b/actioncable/README.md @@ -178,7 +178,7 @@ App.cable.subscriptions.create "AppearanceChannel", ``` Simply calling `App.cable.subscriptions.create` will setup the subscription, which will call `AppearanceChannel#subscribed`, -which in turn is linked to original `App.cable` -> `ApplicationCable::Connection` instances. +which in turn is linked to the original `App.cable` -> `ApplicationCable::Connection` instances. Next, we link the client-side `appear` method to `AppearanceChannel#appear(data)`. This is possible because the server-side channel instance will automatically expose the public methods declared on the class (minus the callbacks), so that these diff --git a/actioncable/lib/action_cable/channel/periodic_timers.rb b/actioncable/lib/action_cable/channel/periodic_timers.rb index b414255707..dab604440f 100644 --- a/actioncable/lib/action_cable/channel/periodic_timers.rb +++ b/actioncable/lib/action_cable/channel/periodic_timers.rb @@ -12,11 +12,42 @@ module ActionCable end module ClassMethods - # Allows you to call a private method <tt>every</tt> so often seconds. This periodic timer can be useful - # for sending a steady flow of updates to a client based off an object that was configured on subscription. - # It's an alternative to using streams if the channel is able to do the work internally. - def periodically(callback, every:) - self.periodic_timers += [ [ callback, every: every ] ] + # Periodically performs a task on the channel, like updating an online + # user counter, polling a backend for new status messages, sending + # regular "heartbeat" messages, or doing some internal work and giving + # progress updates. + # + # Pass a method name or lambda argument or provide a block to call. + # Specify the calling period in seconds using the <tt>every:</tt> + # keyword argument. + # + # periodically :transmit_progress, every: 5.seconds + # + # periodically every: 3.minutes do + # transmit action: :update_count, count: current_count + # end + # + def periodically(callback_or_method_name = nil, every:, &block) + callback = + if block_given? + raise ArgumentError, 'Pass a block or provide a callback arg, not both' if callback_or_method_name + block + else + case callback_or_method_name + when Proc + callback_or_method_name + when Symbol + -> { __send__ callback_or_method_name } + else + raise ArgumentError, "Expected a Symbol method name or a Proc, got #{callback_or_method_name.inspect}" + end + end + + unless every.kind_of?(Numeric) && every > 0 + raise ArgumentError, "Expected every: to be a positive number of seconds, got #{every.inspect}" + end + + self.periodic_timers += [[ callback, every: every ]] end end @@ -27,14 +58,21 @@ module ActionCable def start_periodic_timers self.class.periodic_timers.each do |callback, options| - active_periodic_timers << connection.server.event_loop.timer(options[:every]) do - connection.worker_pool.async_run_periodic_timer(self, callback) + active_periodic_timers << start_periodic_timer(callback, every: options.fetch(:every)) + end + end + + def start_periodic_timer(callback, every:) + connection.server.event_loop.timer every do + connection.worker_pool.async_invoke connection do + instance_exec(&callback) end end end def stop_periodic_timers active_periodic_timers.each { |timer| timer.shutdown } + active_periodic_timers.clear end end end diff --git a/actioncable/lib/action_cable/channel/streams.rb b/actioncable/lib/action_cable/channel/streams.rb index 8b46ac216a..200c9d053c 100644 --- a/actioncable/lib/action_cable/channel/streams.rb +++ b/actioncable/lib/action_cable/channel/streams.rb @@ -2,7 +2,7 @@ module ActionCable module Channel # Streams allow channels to route broadcastings to the subscriber. A broadcasting is, as discussed elsewhere, a pubsub queue where any data # placed into it is automatically sent to the clients that are connected at that time. It's purely an online queue, though. If you're not - # streaming a broadcasting at the very moment it sends out an update, you will not get that update, if you connect after it has been sent. + # streaming a broadcasting at the very moment it sends out an update, you will not get that update, even if you connect after it has been sent. # # Most commonly, the streamed broadcast is sent straight to the subscriber on the client-side. The channel just acts as a connector between # the two parties (the broadcaster and the channel subscriber). Here's an example of a channel that allows subscribers to get all new @@ -73,18 +73,13 @@ module ActionCable # Defaults to `coder: nil` which does no decoding, passes raw messages. def stream_from(broadcasting, callback = nil, coder: nil, &block) broadcasting = String(broadcasting) + # Don't send the confirmation until pubsub#subscribe is successful defer_subscription_confirmation! - if user_handler = callback || block - user_handler = -> message { handler.(coder.decode(message)) } if coder - handler = -> message do - connection.worker_pool.async_invoke(user_handler, :call, message) - end - else - handler = default_stream_handler(broadcasting, coder: coder) - end - + # Build a stream handler by wrapping the user-provided callback with + # a decoder or defaulting to a JSON-decoding retransmitter. + handler = worker_pool_stream_handler(broadcasting, callback || block, coder: coder) streams << [ broadcasting, handler ] connection.server.event_loop.post do @@ -120,13 +115,60 @@ module ActionCable @_streams ||= [] end + # Always wrap the outermost handler to invoke the user handler on the + # worker pool rather than blocking the event loop. + def worker_pool_stream_handler(broadcasting, user_handler, coder: nil) + handler = stream_handler(broadcasting, user_handler, coder: coder) + + -> message do + connection.worker_pool.async_invoke handler, :call, message, connection: connection + end + end + + # May be overridden to add instrumentation, logging, specialized error + # handling, or other forms of handler decoration. + # + # TODO: Tests demonstrating this. + def stream_handler(broadcasting, user_handler, coder: nil) + if user_handler + stream_decoder user_handler, coder: coder + else + default_stream_handler broadcasting, coder: coder + end + end + + # May be overridden to change the default stream handling behavior + # which decodes JSON and transmits to client. + # + # TODO: Tests demonstrating this. + # + # TODO: Room for optimization. Update transmit API to be coder-aware + # so we can no-op when pubsub and connection are both JSON-encoded. + # Then we can skip decode+encode if we're just proxying messages. def default_stream_handler(broadcasting, coder:) coder ||= ActiveSupport::JSON + stream_transmitter stream_decoder(coder: coder), broadcasting: broadcasting + end + + def stream_decoder(handler = identity_handler, coder:) + if coder + -> message { handler.(coder.decode(message)) } + else + handler + end + end + + def stream_transmitter(handler = identity_handler, broadcasting:) + via = "streamed from #{broadcasting}" -> (message) do - transmit coder.decode(message), via: "streamed from #{broadcasting}" + transmit handler.(message), via: via end end + + def identity_handler + -> message { message } + end end end end diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb index 9a7dfbe761..cc4e0f8c8b 100644 --- a/actioncable/lib/action_cable/connection/base.rb +++ b/actioncable/lib/action_cable/connection/base.rb @@ -40,7 +40,7 @@ module ActionCable # Second, we rely on the fact that the WebSocket connection is established with the cookies from the domain being sent along. This makes # it easy to use signed cookies that were set when logging in via a web interface to authorize the WebSocket connection. # - # Finally, we add a tag to the connection-specific logger with name of the current user to easily distinguish their messages in the log. + # Finally, we add a tag to the connection-specific logger with the name of the current user to easily distinguish their messages in the log. # # Pretty simple, eh? class Base diff --git a/actioncable/lib/action_cable/server/worker.rb b/actioncable/lib/action_cable/server/worker.rb index 49cbaec0c0..a638ff72e7 100644 --- a/actioncable/lib/action_cable/server/worker.rb +++ b/actioncable/lib/action_cable/server/worker.rb @@ -12,8 +12,10 @@ module ActionCable define_callbacks :work include ActiveRecordConnectionManagement + attr_reader :executor + def initialize(max_size: 5) - @pool = Concurrent::ThreadPoolExecutor.new( + @executor = Concurrent::ThreadPoolExecutor.new( min_threads: 1, max_threads: max_size, max_queue: 0, @@ -23,11 +25,11 @@ module ActionCable # Stop processing work: any work that has not already started # running will be discarded from the queue def halt - @pool.kill + @executor.kill end def stopping? - @pool.shuttingdown? + @executor.shuttingdown? end def work(connection) @@ -40,14 +42,14 @@ module ActionCable self.connection = nil end - def async_invoke(receiver, method, *args) - @pool.post do - invoke(receiver, method, *args) + def async_invoke(receiver, method, *args, connection: receiver) + @executor.post do + invoke(receiver, method, *args, connection: connection) end end - def invoke(receiver, method, *args) - work(receiver) do + def invoke(receiver, method, *args, connection:) + work(connection) do begin receiver.send method, *args rescue Exception => e @@ -59,18 +61,6 @@ module ActionCable end end - def async_run_periodic_timer(channel, callback) - @pool.post do - run_periodic_timer(channel, callback) - end - end - - def run_periodic_timer(channel, callback) - work(channel.connection) do - callback.respond_to?(:call) ? channel.instance_exec(&callback) : channel.send(callback) - end - end - private def logger diff --git a/actioncable/lib/rails/generators/channel/channel_generator.rb b/actioncable/lib/rails/generators/channel/channel_generator.rb index 3bcf5f1898..05fd21a954 100644 --- a/actioncable/lib/rails/generators/channel/channel_generator.rb +++ b/actioncable/lib/rails/generators/channel/channel_generator.rb @@ -13,7 +13,9 @@ module Rails template "channel.rb", File.join('app/channels', class_path, "#{file_name}_channel.rb") if options[:assets] - template "assets/cable.js", "app/assets/javascripts/cable.js" + if self.behavior == :invoke + template "assets/cable.js", "app/assets/javascripts/cable.js" + end template "assets/channel.coffee", File.join('app/assets/javascripts/channels', class_path, "#{file_name}.coffee") end diff --git a/actioncable/test/channel/periodic_timers_test.rb b/actioncable/test/channel/periodic_timers_test.rb index e6f0c14c9d..03464003cf 100644 --- a/actioncable/test/channel/periodic_timers_test.rb +++ b/actioncable/test/channel/periodic_timers_test.rb @@ -1,12 +1,21 @@ require 'test_helper' require 'stubs/test_connection' require 'stubs/room' +require 'active_support/time' class ActionCable::Channel::PeriodicTimersTest < ActiveSupport::TestCase class ChatChannel < ActionCable::Channel::Base - periodically -> { ping }, every: 5 + # Method name arg periodically :send_updates, every: 1 + # Proc arg + periodically -> { ping }, every: 2 + + # Block arg + periodically every: 3 do + ping + end + private def ping end @@ -19,22 +28,41 @@ class ActionCable::Channel::PeriodicTimersTest < ActiveSupport::TestCase test "periodic timers definition" do timers = ChatChannel.periodic_timers - assert_equal 2, timers.size + assert_equal 3, timers.size - first_timer = timers[0] - assert_kind_of Proc, first_timer[0] - assert_equal 5, first_timer[1][:every] + timers.each_with_index do |timer, i| + assert_kind_of Proc, timer[0] + assert_equal i+1, timer[1][:every] + end + end - second_timer = timers[1] - assert_equal :send_updates, second_timer[0] - assert_equal 1, second_timer[1][:every] + test 'disallow negative and zero periods' do + [ 0, 0.0, 0.seconds, -1, -1.seconds, 'foo', :foo, Object.new ].each do |invalid| + assert_raise ArgumentError, /Expected every:/ do + ChatChannel.periodically :send_updates, every: invalid + end + end + end + + test 'disallow block and arg together' do + assert_raise ArgumentError, /not both/ do + ChatChannel.periodically(:send_updates, every: 1) { ping } + end + end + + test 'disallow unknown args' do + [ 'send_updates', Object.new, nil ].each do |invalid| + assert_raise ArgumentError, /Expected a Symbol/ do + ChatChannel.periodically invalid, every: 1 + end + end end test "timer start and stop" do - @connection.server.event_loop.expects(:timer).times(2).returns(true) + @connection.server.event_loop.expects(:timer).times(3).returns(stub(shutdown: nil)) channel = ChatChannel.new @connection, "{id: 1}", { id: 1 } - channel.expects(:stop_periodic_timers).once channel.unsubscribe_from_channel + assert_equal [], channel.send(:active_periodic_timers) end end diff --git a/actioncable/test/channel/stream_test.rb b/actioncable/test/channel/stream_test.rb index df129a7c62..0b0c72ccf6 100644 --- a/actioncable/test/channel/stream_test.rb +++ b/actioncable/test/channel/stream_test.rb @@ -142,6 +142,7 @@ module ActionCable::StreamTests connection.websocket.expects(:transmit) @server.broadcast 'test_room_1', { foo: 'bar' }, coder: DummyEncoder wait_for_async + wait_for_executor connection.server.worker_pool.executor end end diff --git a/actioncable/test/client_test.rb b/actioncable/test/client_test.rb index 5ac453db35..fe503fd703 100644 --- a/actioncable/test/client_test.rb +++ b/actioncable/test/client_test.rb @@ -226,7 +226,9 @@ class ClientTest < ActionCable::TestCase assert_equal(1, app.connections.count) assert(app.remote_connections.where(identifier: identifier)) - channel = app.connections.first.subscriptions.send(:subscriptions).first[1] + subscriptions = app.connections.first.subscriptions.send(:subscriptions) + assert_not_equal 0, subscriptions.size, 'Missing EchoChannel subscription' + channel = subscriptions.first[1] channel.expects(:unsubscribed) c.close sleep 0.1 # Data takes a moment to process diff --git a/actioncable/test/test_helper.rb b/actioncable/test/test_helper.rb index de1ee96770..0a9ee7ce77 100644 --- a/actioncable/test/test_helper.rb +++ b/actioncable/test/test_helper.rb @@ -49,10 +49,7 @@ end module ConcurrentRubyConcurrencyHelpers def wait_for_async - e = Concurrent.global_io_executor - until e.completed_task_count == e.scheduled_task_count - sleep 0.1 - end + wait_for_executor Concurrent.global_io_executor end def run_in_eventmachine @@ -67,4 +64,10 @@ class ActionCable::TestCase < ActiveSupport::TestCase else include ConcurrentRubyConcurrencyHelpers end + + def wait_for_executor(executor) + until executor.completed_task_count == executor.scheduled_task_count + sleep 0.1 + end + end end diff --git a/actioncable/test/worker_test.rb b/actioncable/test/worker_test.rb index 654f49821e..e2c81fe312 100644 --- a/actioncable/test/worker_test.rb +++ b/actioncable/test/worker_test.rb @@ -33,22 +33,12 @@ class WorkerTest < ActiveSupport::TestCase end test "invoke" do - @worker.invoke @receiver, :run + @worker.invoke @receiver, :run, connection: @receiver.connection assert_equal :run, @receiver.last_action end test "invoke with arguments" do - @worker.invoke @receiver, :process, "Hello" + @worker.invoke @receiver, :process, "Hello", connection: @receiver.connection assert_equal [ :process, "Hello" ], @receiver.last_action end - - test "running periodic timers with a proc" do - @worker.run_periodic_timer @receiver, @receiver.method(:run) - assert_equal :run, @receiver.last_action - end - - test "running periodic timers with a method" do - @worker.run_periodic_timer @receiver, :run - assert_equal :run, @receiver.last_action - end end diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index 957e7a3019..59984a0028 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -84,7 +84,7 @@ module ActionController #:nodoc: # Options: # * <tt>:filename</tt> - suggests a filename for the browser to use. # * <tt>:type</tt> - specifies an HTTP content type. Defaults to 'application/octet-stream'. You can specify - # either a string or a symbol for a registered type register with <tt>Mime::Type.register</tt>, for example :json + # either a string or a symbol for a registered type register with <tt>Mime::Type.register</tt>, for example :json. # If omitted, type will be guessed from the file extension specified in <tt>:filename</tt>. # If no content type is registered for the extension, default type 'application/octet-stream' will be used. # * <tt>:disposition</tt> - specifies whether the file will be shown inline or downloaded. diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 754ac144cc..7faf3cd8c6 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -219,12 +219,15 @@ CACHED end def test_fragment_caching_with_options + time = Time.now get :fragment_cached_with_options assert_response :success expected_body = "<body>\n<p>ERB</p>\n</body>\n" assert_equal expected_body, @response.body - assert_equal "<p>ERB</p>", @store.read("views/with_options") + Time.stub(:now, time + 11) do + assert_nil @store.read("views/with_options") + end end def test_render_inline_before_fragment_caching diff --git a/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb b/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb index 01453323ef..951c761995 100644 --- a/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb +++ b/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb @@ -1,3 +1,3 @@ <body> -<%= cache 'with_options', skip_digest: true, expires_in: 1.minute do %><p>ERB</p><% end %> +<%= cache 'with_options', skip_digest: true, expires_in: 10 do %><p>ERB</p><% end %> </body> diff --git a/actionview/test/fixtures/test/_🍣.erb b/actionview/test/fixtures/test/_🍣.erb new file mode 100644 index 0000000000..4bbe59410a --- /dev/null +++ b/actionview/test/fixtures/test/_🍣.erb @@ -0,0 +1 @@ +🍣 diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index b417d1ebfa..ad93236d32 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -207,6 +207,10 @@ module RenderTestCases assert_nothing_raised { @view.render(:partial => "test/a-in") } end + def test_render_partial_with_unicode_text + assert_nothing_raised { @view.render(:partial => "test/🍣") } + end + def test_render_partial_with_invalid_option_as e = assert_raises(ArgumentError) { @view.render(:partial => "test/partial_only", :as => 'a-in') } assert_equal "The value (a-in) of the option `as` is not a valid Ruby identifier; " + diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index e6cbd6bd68..16ce131cf4 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,78 @@ +* SQLite: Fix uniqueness validation when values exceed the column limit. + + SQLite doesn't impose length restrictions on strings, BLOBs, or numeric + values. It treats them as helpful metadata. When we truncate strings + before checking uniqueness, we'd miss values that exceed the column limit. + + Other databases enforce length limits. A large value will pass uniqueness + validation since the column limit guarantees no value that long exists. + When we insert the row, it'll raise `ActiveRecord::ValueTooLong` as we + expect. + + This fixes edge-case incorrect validation failures for values that exceed + the column limit but are identical to an existing value *when truncated*. + Now these will pass validation and raise an exception. + + *Ryuta Kamizono* + +* Raise `ActiveRecord::ValueTooLong` when column limits are exceeded. + Supported by MySQL and PostgreSQL adapters. + + *Ryuta Kamizono* + +* Migrations: `#foreign_key` respects `table_name_prefix` and `_suffix`. + + *Ryuta Kamizono* + +* SQLite: Force NOT NULL primary keys. + + From SQLite docs: https://www.sqlite.org/lang_createtable.html + According to the SQL standard, PRIMARY KEY should always imply NOT + NULL. Unfortunately, due to a bug in some early versions, this is not + the case in SQLite. Unless the column is an INTEGER PRIMARY KEY or the + table is a WITHOUT ROWID table or the column is declared NOT NULL, + SQLite allows NULL values in a PRIMARY KEY column. SQLite could be + fixed to conform to the standard, but doing so might break legacy + applications. Hence, it has been decided to merely document the fact + that SQLite allowing NULLs in most PRIMARY KEY columns. + + Now we override column options to explicitly set NOT NULL rather than rely + on implicit NOT NULL like MySQL and PostgreSQL adapters. + + *Ryuta Kamizono* + +* Added notice when a database is successfully created or dropped. + + Example: + + $ bin/rails db:create + Created database 'blog_development' + Created database 'blog_test' + + $ bin/rails db:drop + Dropped database 'blog_development' + Dropped database 'blog_test' + + Changed older notices + `blog_development already exists` to `Database 'blog_development' already exists`. + and + `Couldn't drop blog_development` to `Couldn't drop database 'blog_development'`. + + *bogdanvlviv* + +* Database comments. Annotate database objects (tables, columns, indexes) + with comments stored in database metadata. PostgreSQL & MySQL support. + + create_table :pages, force: :cascade, comment: 'CMS content pages' do |t| + t.string :path, comment: 'Path fragment of page URL used for routing' + t.string :locale, comment: 'RFC 3066 locale code of website language section' + t.index [:path, :locale], comment: 'Look up pages by URI' + end + + *Andrey Novikov* + * Add `quoted_time` for truncating the date part of a TIME column value. - This fixes queries on TIME column on MariaDB, as it doesn't ignore the + This fixes queries on TIME column on MariaDB, as it doesn't ignore the date part of the string when it coerces to time. *Ryuta Kamizono* diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 77d17fc975..2fbecb7d04 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1326,7 +1326,8 @@ module ActiveRecord # Specifies type of the source association used by #has_many <tt>:through</tt> queries where the source # association is a polymorphic #belongs_to. # [:validate] - # If +false+, don't validate the associated objects when saving the parent object. true by default. + # When set to +true+, validates new objects added to association when saving the parent object. +true+ by default. + # If you want to ensure associated objects are revalidated on every update, use +validates_associated+. # [:autosave] # If true, always save the associated objects or destroy them if marked for destruction, # when saving the parent object. If false, never save or destroy the associated objects. @@ -1456,7 +1457,8 @@ module ActiveRecord # Specifies type of the source association used by #has_one <tt>:through</tt> queries where the source # association is a polymorphic #belongs_to. # [:validate] - # If +false+, don't validate the associated object when saving the parent object. +false+ by default. + # When set to +true+, validates new objects added to association when saving the parent object. +false+ by default. + # If you want to ensure associated objects are revalidated on every update, use +validates_associated+. # [:autosave] # If true, always save the associated object or destroy it if marked for destruction, # when saving the parent object. If false, never save or destroy the associated object. @@ -1580,7 +1582,8 @@ module ActiveRecord # Note: If you've enabled the counter cache, then you may want to add the counter cache attribute # to the +attr_readonly+ list in the associated classes (e.g. <tt>class Post; attr_readonly :comments_count; end</tt>). # [:validate] - # If +false+, don't validate the associated objects when saving the parent object. +false+ by default. + # When set to +true+, validates new objects added to association when saving the parent object. +false+ by default. + # If you want to ensure associated objects are revalidated on every update, use +validates_associated+. # [:autosave] # If true, always save the associated object or destroy it if marked for destruction, when # saving the parent object. @@ -1766,7 +1769,8 @@ module ActiveRecord # So if a Person class makes a #has_and_belongs_to_many association to Project, # the association will use "project_id" as the default <tt>:association_foreign_key</tt>. # [:validate] - # If +false+, don't validate the associated objects when saving the parent object. +true+ by default. + # When set to +true+, validates new objects added to association when saving the parent object. +true+ by default. + # If you want to ensure associated objects are revalidated on every update, use +validates_associated+. # [:autosave] # If true, always save the associated objects or destroy them if marked for destruction, when # saving the parent object. diff --git a/activerecord/lib/active_record/collection_cache_key.rb b/activerecord/lib/active_record/collection_cache_key.rb index 5dcc98424a..8d41c0d799 100644 --- a/activerecord/lib/active_record/collection_cache_key.rb +++ b/activerecord/lib/active_record/collection_cache_key.rb @@ -16,7 +16,7 @@ module ActiveRecord query = collection .unscope(:select) - .select("COUNT(*) AS size", "MAX(#{column}) AS timestamp") + .select("COUNT(*) AS #{connection.quote_column_name("size")}", "MAX(#{column}) AS timestamp") .unscope(:order) result = connection.select_one(query) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb index 0ba4d94e3c..6add697eeb 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb @@ -53,8 +53,8 @@ module ActiveRecord statements.concat(o.foreign_keys.map { |to_table, options| foreign_key_in_create(o.name, to_table, options) }) end - create_sql << "(#{statements.join(', ')}) " if statements.present? - create_sql << "#{o.options}" + create_sql << "(#{statements.join(', ')})" if statements.present? + add_table_options!(create_sql, table_options(o)) create_sql << " AS #{@conn.to_sql(o.as)}" if o.as create_sql end @@ -82,6 +82,19 @@ module ActiveRecord "DROP CONSTRAINT #{quote_column_name(name)}" end + def table_options(o) + table_options = {} + table_options[:comment] = o.comment + table_options[:options] = o.options + table_options + end + + def add_table_options!(create_sql, options) + if options_sql = options[:options] + create_sql << " #{options_sql}" + end + end + def column_options(o) column_options = {} column_options[:null] = o.null unless o.null.nil? @@ -92,6 +105,7 @@ module ActiveRecord column_options[:auto_increment] = o.auto_increment column_options[:primary_key] = o.primary_key column_options[:collation] = o.collation + column_options[:comment] = o.comment column_options end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 4f97c7c065..bbb0e9249d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -3,14 +3,14 @@ module ActiveRecord # Abstract representation of an index definition on a table. Instances of # this type are typically created and returned by methods in database # adapters. e.g. ActiveRecord::ConnectionAdapters::AbstractMysqlAdapter#indexes - class IndexDefinition < Struct.new(:table, :name, :unique, :columns, :lengths, :orders, :where, :type, :using) #:nodoc: + class IndexDefinition < Struct.new(:table, :name, :unique, :columns, :lengths, :orders, :where, :type, :using, :comment) #:nodoc: end # Abstract representation of a column definition. Instances of this type # are typically created by methods in TableDefinition, and added to the # +columns+ attribute of said TableDefinition object, in order to be used # for generating a number of table creation or table changing SQL statements. - class ColumnDefinition < Struct.new(:name, :type, :limit, :precision, :scale, :default, :null, :first, :after, :auto_increment, :primary_key, :collation, :sql_type) #:nodoc: + class ColumnDefinition < Struct.new(:name, :type, :limit, :precision, :scale, :default, :null, :first, :after, :auto_increment, :primary_key, :collation, :sql_type, :comment) #:nodoc: def primary_key? primary_key || type.to_sym == :primary_key @@ -207,9 +207,9 @@ module ActiveRecord include ColumnMethods attr_accessor :indexes - attr_reader :name, :temporary, :options, :as, :foreign_keys + attr_reader :name, :temporary, :options, :as, :foreign_keys, :comment - def initialize(name, temporary, options, as = nil) + def initialize(name, temporary = false, options = nil, as = nil, comment: nil) @columns_hash = {} @indexes = {} @foreign_keys = [] @@ -218,6 +218,7 @@ module ActiveRecord @options = options @as = as @name = name + @comment = comment end def primary_keys(name = nil) # :nodoc: @@ -330,6 +331,9 @@ module ActiveRecord end def foreign_key(table_name, options = {}) # :nodoc: + table_name_prefix = ActiveRecord::Base.table_name_prefix + table_name_suffix = ActiveRecord::Base.table_name_suffix + table_name = "#{table_name_prefix}#{table_name}#{table_name_suffix}" foreign_keys.push([table_name, options]) end @@ -373,6 +377,7 @@ module ActiveRecord column.auto_increment = options[:auto_increment] column.primary_key = type == :primary_key || options[:primary_key] column.collation = options[:collation] + column.comment = options[:comment] column end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb index 4880d216d6..6b5fad00ab 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb @@ -16,7 +16,7 @@ module ActiveRecord def column_spec_for_primary_key(column) return {} if default_primary_key?(column) spec = { id: schema_type(column).inspect } - spec.merge!(prepare_column_options(column)) + spec.merge!(prepare_column_options(column).except!(:null)) end # This can be overridden on an Adapter level basis to support other @@ -46,12 +46,14 @@ module ActiveRecord spec[:collation] = collation end + spec[:comment] = column.comment.inspect if column.comment + spec end # Lists the valid migration options def migration_keys - [:name, :limit, :precision, :scale, :default, :null, :collation] + [:name, :limit, :precision, :scale, :default, :null, :collation, :comment] end private diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 020d9bbdca..4f361fed6b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -18,6 +18,11 @@ module ActiveRecord nil end + # Returns the table comment that's stored in database metadata. + def table_comment(table_name) + nil + end + # Truncates a table alias according to the limits of the current adapter. def table_alias_for(table_name) table_name[0...table_alias_length].tr('.', '_') @@ -254,8 +259,8 @@ module ActiveRecord # SELECT * FROM orders INNER JOIN line_items ON order_id=orders.id # # See also TableDefinition#column for details on how to create columns. - def create_table(table_name, options = {}) - td = create_table_definition table_name, options[:temporary], options[:options], options[:as] + def create_table(table_name, comment: nil, **options) + td = create_table_definition table_name, options[:temporary], options[:options], options[:as], comment: comment if options[:id] != false && !options[:as] pk = options.fetch(:primary_key) do @@ -283,6 +288,14 @@ module ActiveRecord end end + if supports_comments? && !supports_comments_in_create? + change_table_comment(table_name, comment) if comment + + td.columns.each do |column| + change_column_comment(table_name, column.name, column.comment) if column.comment + end + end + result end @@ -776,7 +789,8 @@ module ActiveRecord # [<tt>:type</tt>] # The reference column type. Defaults to +:integer+. # [<tt>:index</tt>] - # Add an appropriate index. Defaults to false. + # Add an appropriate index. Defaults to false. + # See #add_index for usage of this option. # [<tt>:foreign_key</tt>] # Add an appropriate foreign key constraint. Defaults to false. # [<tt>:polymorphic</tt>] @@ -796,6 +810,14 @@ module ActiveRecord # # add_reference(:products, :supplier, polymorphic: true, index: true) # + # ====== Create a supplier_id column with a unique index + # + # add_reference(:products, :supplier, index: { unique: true }) + # + # ====== Create a supplier_id column with a named index + # + # add_reference(:products, :supplier, index: { name: "my_supplier_index" }) + # # ====== Create a supplier_id column and appropriate foreign key # # add_reference(:products, :supplier, foreign_key: true) @@ -1075,7 +1097,7 @@ module ActiveRecord Table.new(table_name, base) end - def add_index_options(table_name, column_name, options = {}) #:nodoc: + def add_index_options(table_name, column_name, comment: nil, **options) #:nodoc: column_names = Array(column_name) options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type) @@ -1106,13 +1128,23 @@ module ActiveRecord end index_columns = quoted_columns_for_index(column_names, options).join(", ") - [index_name, index_type, index_columns, index_options, algorithm, using] + [index_name, index_type, index_columns, index_options, algorithm, using, comment] end def options_include_default?(options) options.include?(:default) && !(options[:null] == false && options[:default].nil?) end + # Changes the comment for a table or removes it if +nil+. + def change_table_comment(table_name, comment) + raise NotImplementedError, "#{self.class} does not support changing table comments" + end + + # Changes the comment for a column or removes it if +nil+. + def change_column_comment(table_name, column_name, comment) #:nodoc: + raise NotImplementedError, "#{self.class} does not support changing column comments" + end + protected def add_index_sort_order(option_strings, column_names, options = {}) if options.is_a?(Hash) && order = options[:order] @@ -1194,8 +1226,8 @@ module ActiveRecord end private - def create_table_definition(name, temporary = false, options = nil, as = nil) - TableDefinition.new(name, temporary, options, as) + def create_table_definition(*args) + TableDefinition.new(*args) end def create_alter_table(name) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 069346253a..7276b8de44 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -104,9 +104,15 @@ module ActiveRecord @config = config @pool = nil @schema_cache = SchemaCache.new self - @visitor = nil - @prepared_statements = false @quoted_column_names, @quoted_table_names = {}, {} + @visitor = arel_visitor + + if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) + @prepared_statements = true + @visitor.extend(DetermineIfPreparableVisitor) + else + @prepared_statements = false + end end class Version @@ -142,6 +148,10 @@ module ActiveRecord end end + def arel_visitor # :nodoc: + (Arel::Visitors::VISITORS[@config[:adapter]] || Arel::Visitors::ToSql).new(self) + end + def valid_type?(type) true end @@ -278,6 +288,16 @@ module ActiveRecord false end + # Does this adapter support metadata comments on database objects (tables, columns, indexes)? + def supports_comments? + false + end + + # Can comments for tables, columns, and indexes be specified in create/alter table statements? + def supports_comments_in_create? + false + end + # This is meant to be implemented by the adapters that support extensions def disable_extension(name) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 8bd9da4fbc..5125a0f06e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -55,15 +55,6 @@ module ActiveRecord def initialize(connection, logger, connection_options, config) super(connection, logger, config) - @visitor = Arel::Visitors::MySQL.new self - - if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) - @prepared_statements = true - @visitor.extend(DetermineIfPreparableVisitor) - else - @prepared_statements = false - end - if version < '5.0.0' raise "Your version of MySQL (#{full_version.match(/^\d+\.\d+\.\d+/)[0]}) is too old. Active Record supports MySQL >= 5.0." end @@ -160,8 +151,8 @@ module ActiveRecord raise NotImplementedError end - def new_column(field, default, sql_type_metadata, null, table_name, default_function = nil, collation = nil) # :nodoc: - MySQL::Column.new(field, default, sql_type_metadata, null, table_name, default_function, collation) + def new_column(*args) #:nodoc: + MySQL::Column.new(*args) end # Must return the MySQL error number from the exception, if the exception has an @@ -335,8 +326,7 @@ module ActiveRecord def data_source_exists?(table_name) return false unless table_name.present? - schema, name = table_name.to_s.split('.', 2) - schema, name = @config[:database], schema unless name # A table was provided without a schema + schema, name = extract_schema_qualified_name(table_name) sql = "SELECT table_name FROM information_schema.tables " sql << "WHERE table_schema = #{quote(schema)} AND table_name = #{quote(name)}" @@ -351,8 +341,7 @@ module ActiveRecord def view_exists?(view_name) # :nodoc: return false unless view_name.present? - schema, name = view_name.to_s.split('.', 2) - schema, name = @config[:database], schema unless name # A view was provided without a schema + schema, name = extract_schema_qualified_name(view_name) sql = "SELECT table_name FROM information_schema.tables WHERE table_type = 'VIEW'" sql << " AND table_schema = #{quote(schema)} AND table_name = #{quote(name)}" @@ -373,7 +362,7 @@ module ActiveRecord mysql_index_type = row[:Index_type].downcase.to_sym index_type = INDEX_TYPES.include?(mysql_index_type) ? mysql_index_type : nil index_using = INDEX_USINGS.include?(mysql_index_type) ? mysql_index_type : nil - indexes << IndexDefinition.new(row[:Table], row[:Key_name], row[:Non_unique].to_i == 0, [], [], nil, nil, index_type, index_using) + indexes << IndexDefinition.new(row[:Table], row[:Key_name], row[:Non_unique].to_i == 0, [], [], nil, nil, index_type, index_using, row[:Index_comment].presence) end indexes.last.columns << row[:Column_name] @@ -394,12 +383,20 @@ module ActiveRecord else default, default_function = field[:Default], nil end - new_column(field[:Field], default, type_metadata, field[:Null] == "YES", table_name, default_function, field[:Collation]) + new_column(field[:Field], default, type_metadata, field[:Null] == "YES", table_name, default_function, field[:Collation], comment: field[:Comment].presence) end end - def create_table(table_name, options = {}) #:nodoc: - super(table_name, options.reverse_merge(:options => "ENGINE=InnoDB")) + def table_comment(table_name) # :nodoc: + select_value(<<-SQL.strip_heredoc, 'SCHEMA') + SELECT table_comment + FROM information_schema.tables + WHERE table_name=#{quote(table_name)} + SQL + end + + def create_table(table_name, **options) #:nodoc: + super(table_name, options: 'ENGINE=InnoDB', **options) end def bulk_change_table(table_name, operations) #:nodoc: @@ -482,11 +479,17 @@ module ActiveRecord end def add_index(table_name, column_name, options = {}) #:nodoc: - index_name, index_type, index_columns, _, index_algorithm, index_using = add_index_options(table_name, column_name, options) - execute "CREATE #{index_type} INDEX #{quote_column_name(index_name)} #{index_using} ON #{quote_table_name(table_name)} (#{index_columns}) #{index_algorithm}" + index_name, index_type, index_columns, _, index_algorithm, index_using, comment = add_index_options(table_name, column_name, options) + sql = "CREATE #{index_type} INDEX #{quote_column_name(index_name)} #{index_using} ON #{quote_table_name(table_name)} (#{index_columns}) #{index_algorithm}" + sql << " COMMENT #{quote(comment)}" if comment + execute sql end def foreign_keys(table_name) + raise ArgumentError unless table_name.present? + + schema, name = extract_schema_qualified_name(table_name) + fk_info = select_all <<-SQL.strip_heredoc SELECT fk.referenced_table_name as 'to_table' ,fk.referenced_column_name as 'primary_key' @@ -494,8 +497,8 @@ module ActiveRecord ,fk.constraint_name as 'name' FROM information_schema.key_column_usage fk WHERE fk.referenced_column_name is not null - AND fk.table_schema = '#{@config[:database]}' - AND fk.table_name = '#{table_name}' + AND fk.table_schema = #{quote(schema)} + AND fk.table_name = #{quote(name)} SQL create_table_info = create_table_info(table_name) @@ -521,7 +524,12 @@ module ActiveRecord raw_table_options = create_table_info.sub(/\A.*\n\) /m, '').sub(/\n\/\*!.*\*\/\n\z/m, '').strip # strip AUTO_INCREMENT - raw_table_options.sub(/(ENGINE=\w+)(?: AUTO_INCREMENT=\d+)/, '\1') + raw_table_options.sub!(/(ENGINE=\w+)(?: AUTO_INCREMENT=\d+)/, '\1') + + # strip COMMENT + raw_table_options.sub!(/ COMMENT='.+'/, '') + + raw_table_options end # Maps logical Rails types to MySQL-specific data types. @@ -557,8 +565,7 @@ module ActiveRecord def primary_keys(table_name) # :nodoc: raise ArgumentError unless table_name.present? - schema, name = table_name.to_s.split('.', 2) - schema, name = @config[:database], schema unless name # A table was provided without a schema + schema, name = extract_schema_qualified_name(table_name) select_values(<<-SQL.strip_heredoc, 'SCHEMA') SELECT column_name @@ -701,6 +708,8 @@ module ActiveRecord RecordNotUnique.new(message) when 1452 InvalidForeignKey.new(message) + when 1406 + ValueTooLong.new(message) else super end @@ -866,8 +875,14 @@ module ActiveRecord create_table_info_cache[table_name] ||= select_one("SHOW CREATE TABLE #{quote_table_name(table_name)}")["Create Table"] end - def create_table_definition(name, temporary = false, options = nil, as = nil) # :nodoc: - MySQL::TableDefinition.new(name, temporary, options, as) + def create_table_definition(*args) # :nodoc: + MySQL::TableDefinition.new(*args) + end + + def extract_schema_qualified_name(string) # :nodoc: + schema, name = string.to_s.scan(/[^`.\s]+|`[^`]*`/) + schema, name = @config[:database], schema unless name + [schema, name] end def integer_to_sql(limit) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index 2e718b29fa..28f0c8686a 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -1,11 +1,9 @@ -require 'set' - module ActiveRecord # :stopdoc: module ConnectionAdapters # An abstract definition of a column in a table. class Column - attr_reader :name, :default, :sql_type_metadata, :null, :table_name, :default_function, :collation + attr_reader :name, :default, :sql_type_metadata, :null, :table_name, :default_function, :collation, :comment delegate :precision, :scale, :limit, :type, :sql_type, to: :sql_type_metadata, allow_nil: true @@ -15,7 +13,7 @@ module ActiveRecord # +default+ is the type-casted default value, such as +new+ in <tt>sales_stage varchar(20) default 'new'</tt>. # +sql_type_metadata+ is various information about the type of the column # +null+ determines if this column allows +NULL+ values. - def initialize(name, default, sql_type_metadata = nil, null = true, table_name = nil, default_function = nil, collation = nil) + def initialize(name, default, sql_type_metadata = nil, null = true, table_name = nil, default_function = nil, collation = nil, comment: nil) @name = name.freeze @table_name = table_name @sql_type_metadata = sql_type_metadata @@ -23,6 +21,7 @@ module ActiveRecord @default = default @default_function = default_function @collation = collation + @comment = comment end def has_default? diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb index 1e2c859af9..0384079da2 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb @@ -2,6 +2,9 @@ module ActiveRecord module ConnectionAdapters module MySQL class SchemaCreation < AbstractAdapter::SchemaCreation + delegate :quote, to: :@conn + private :quote + private def visit_DropForeignKey(name) @@ -22,6 +25,14 @@ module ActiveRecord add_column_position!(change_column_sql, column_options(o.column)) end + def add_table_options!(create_sql, options) + super + + if comment = options[:comment] + create_sql << " COMMENT #{quote(comment)}" + end + end + def column_options(o) column_options = super column_options[:charset] = o.charset @@ -29,13 +40,21 @@ module ActiveRecord end def add_column_options!(sql, options) - if options[:charset] - sql << " CHARACTER SET #{options[:charset]}" + if charset = options[:charset] + sql << " CHARACTER SET #{charset}" end - if options[:collation] - sql << " COLLATE #{options[:collation]}" + + if collation = options[:collation] + sql << " COLLATE #{collation}" end + super + + if comment = options[:comment] + sql << " COMMENT #{quote(comment)}" + end + + sql end def add_column_position!(sql, options) @@ -44,12 +63,14 @@ module ActiveRecord elsif options[:after] sql << " AFTER #{quote_column_name(options[:after])}" end + sql end def index_in_create(table_name, column_name, options) - index_name, index_type, index_columns, _, _, index_using = @conn.add_index_options(table_name, column_name, options) - "#{index_type} INDEX #{quote_column_name(index_name)} #{index_using} (#{index_columns}) " + index_name, index_type, index_columns, _, _, index_using, comment = @conn.add_index_options(table_name, column_name, options) + index_option = " COMMENT #{quote(comment)}" if comment + "#{index_type} INDEX #{quote_column_name(index_name)} #{index_using} (#{index_columns})#{index_option} " end end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb index be40df4101..2ba9657f24 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb @@ -7,7 +7,7 @@ module ActiveRecord spec = { id: :bigint.inspect } spec[:default] = schema_default(column) || 'nil' unless column.auto_increment? else - spec = super.except!(:null) + spec = super end spec[:unsigned] = 'true' if column.unsigned? spec diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index e7541748de..ec343a5a57 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -45,6 +45,14 @@ module ActiveRecord !mariadb? && version >= '5.7.8' end + def supports_comments? + true + end + + def supports_comments_in_create? + true + end + # HELPER METHODS =========================================== def each_hash(result) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb index 1047ba8cac..a1e10fd364 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb @@ -3,7 +3,7 @@ module ActiveRecord module PostgreSQL module ColumnDumper def column_spec_for_primary_key(column) - spec = super.except!(:null) + spec = super if schema_type(column) == :uuid spec[:default] ||= 'nil' end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 39e8492688..4a66b82cbb 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/string/strip' + module ActiveRecord module ConnectionAdapters module PostgreSQL @@ -172,7 +174,8 @@ module ActiveRecord table = Utils.extract_schema_qualified_name(table_name.to_s) result = query(<<-SQL, 'SCHEMA') - SELECT distinct i.relname, d.indisunique, d.indkey, pg_get_indexdef(d.indexrelid), t.oid + SELECT distinct i.relname, d.indisunique, d.indkey, pg_get_indexdef(d.indexrelid), t.oid, + pg_catalog.obj_description(i.oid, 'pg_class') AS comment FROM pg_class t INNER JOIN pg_index d ON t.oid = d.indrelid INNER JOIN pg_class i ON d.indexrelid = i.oid @@ -190,6 +193,7 @@ module ActiveRecord indkey = row[2].split(" ").map(&:to_i) inddef = row[3] oid = row[4] + comment = row[5] columns = Hash[query(<<-SQL, "SCHEMA")] SELECT a.attnum, a.attname @@ -207,7 +211,7 @@ module ActiveRecord where = inddef.scan(/WHERE (.+)$/).flatten[0] using = inddef.scan(/USING (.+?) /).flatten[0].to_sym - IndexDefinition.new(table_name, index_name, unique, column_names, [], orders, where, nil, using) + IndexDefinition.new(table_name, index_name, unique, column_names, [], orders, where, nil, using, comment) end end.compact end @@ -215,18 +219,33 @@ module ActiveRecord # Returns the list of all column definitions for a table. def columns(table_name) # :nodoc: table_name = table_name.to_s - column_definitions(table_name).map do |column_name, type, default, notnull, oid, fmod, collation| + column_definitions(table_name).map do |column_name, type, default, notnull, oid, fmod, collation, comment| oid = oid.to_i fmod = fmod.to_i type_metadata = fetch_type_metadata(column_name, type, oid, fmod) default_value = extract_value_from_default(default) default_function = extract_default_function(default_value, default) - new_column(column_name, default_value, type_metadata, !notnull, table_name, default_function, collation) + new_column(column_name, default_value, type_metadata, !notnull, table_name, default_function, collation, comment: comment) end end - def new_column(name, default, sql_type_metadata, null, table_name, default_function = nil, collation = nil) # :nodoc: - PostgreSQLColumn.new(name, default, sql_type_metadata, null, table_name, default_function, collation) + def new_column(*args) # :nodoc: + PostgreSQLColumn.new(*args) + end + + # Returns a comment stored in database for given table + def table_comment(table_name) # :nodoc: + name = Utils.extract_schema_qualified_name(table_name.to_s) + if name.identifier + select_value(<<-SQL.strip_heredoc, 'SCHEMA') + SELECT pg_catalog.obj_description(c.oid, 'pg_class') + FROM pg_catalog.pg_class c + LEFT JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = #{quote(name.identifier)} + AND c.relkind IN ('r') -- (r)elation/table + AND n.nspname = #{name.schema ? quote(name.schema) : 'ANY (current_schemas(false))'} + SQL + end end # Returns the current database name. @@ -445,6 +464,7 @@ module ActiveRecord def add_column(table_name, column_name, type, options = {}) #:nodoc: clear_cache! super + change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) end def change_column(table_name, column_name, type, options = {}) #:nodoc: @@ -466,6 +486,7 @@ module ActiveRecord change_column_default(table_name, column_name, options[:default]) if options_include_default?(options) change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) end # Changes the default value of a table column. @@ -494,6 +515,18 @@ module ActiveRecord execute("ALTER TABLE #{quote_table_name(table_name)} ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL") end + # Adds comment for given table column or drops it if +comment+ is a +nil+ + def change_column_comment(table_name, column_name, comment) # :nodoc: + clear_cache! + execute "COMMENT ON COLUMN #{quote_table_name(table_name)}.#{quote_column_name(column_name)} IS #{quote(comment)}" + end + + # Adds comment for given table or drops it if +comment+ is a +nil+ + def change_table_comment(table_name, comment) # :nodoc: + clear_cache! + execute "COMMENT ON TABLE #{quote_table_name(table_name)} IS #{quote(comment)}" + end + # Renames a column in a table. def rename_column(table_name, column_name, new_column_name) #:nodoc: clear_cache! @@ -502,8 +535,10 @@ module ActiveRecord end def add_index(table_name, column_name, options = {}) #:nodoc: - index_name, index_type, index_columns, index_options, index_algorithm, index_using = add_index_options(table_name, column_name, options) - execute "CREATE #{index_type} INDEX #{index_algorithm} #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} #{index_using} (#{index_columns})#{index_options}" + index_name, index_type, index_columns, index_options, index_algorithm, index_using, comment = add_index_options(table_name, column_name, options) + execute("CREATE #{index_type} INDEX #{index_algorithm} #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} #{index_using} (#{index_columns})#{index_options}").tap do + execute "COMMENT ON INDEX #{quote_column_name(index_name)} IS #{quote(comment)}" if comment + end end def remove_index(table_name, options = {}) #:nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 6497b1cc31..a052b33627 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -159,6 +159,14 @@ module ActiveRecord postgresql_version >= 90200 end + def supports_comments? + true + end + + def supports_comments_in_create? + false + end + def index_algorithms { concurrently: 'CONCURRENTLY' } end @@ -195,14 +203,6 @@ module ActiveRecord def initialize(connection, logger, connection_parameters, config) super(connection, logger, config) - @visitor = Arel::Visitors::PostgreSQL.new self - if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) - @prepared_statements = true - @visitor.extend(DetermineIfPreparableVisitor) - else - @prepared_statements = false - end - @connection_parameters = connection_parameters # @local_tz is initialized as nil to avoid warnings when connect tries to use it @@ -398,6 +398,7 @@ module ActiveRecord protected # See http://www.postgresql.org/docs/current/static/errcodes-appendix.html + VALUE_LIMIT_VIOLATION = "22001" FOREIGN_KEY_VIOLATION = "23503" UNIQUE_VIOLATION = "23505" @@ -409,6 +410,8 @@ module ActiveRecord RecordNotUnique.new(message) when FOREIGN_KEY_VIOLATION InvalidForeignKey.new(message) + when VALUE_LIMIT_VIOLATION + ValueTooLong.new(message) else super end @@ -712,7 +715,7 @@ module ActiveRecord # Returns the list of a table's column names, data types, and default values. # # The underlying query is roughly: - # SELECT column.name, column.type, default.value + # SELECT column.name, column.type, default.value, column.comment # FROM column LEFT JOIN default # ON column.table_id = default.table_id # AND column.num = default.column_num @@ -732,7 +735,8 @@ module ActiveRecord SELECT a.attname, format_type(a.atttypid, a.atttypmod), pg_get_expr(d.adbin, d.adrelid), a.attnotnull, a.atttypid, a.atttypmod, (SELECT c.collname FROM pg_collation c, pg_type t - WHERE c.oid = a.attcollation AND t.oid = a.atttypid AND a.attcollation <> t.typcollation) + WHERE c.oid = a.attcollation AND t.oid = a.atttypid AND a.attcollation <> t.typcollation), + col_description(a.attrelid, a.attnum) AS comment FROM pg_attribute a LEFT JOIN pg_attrdef d ON a.attrelid = d.adrelid AND a.attnum = d.adnum WHERE a.attrelid = '#{quote_table_name(table_name)}'::regclass @@ -746,8 +750,8 @@ module ActiveRecord $1.strip if $1 end - def create_table_definition(name, temporary = false, options = nil, as = nil) # :nodoc: - PostgreSQL::TableDefinition.new(name, temporary, options, as) + def create_table_definition(*args) # :nodoc: + PostgreSQL::TableDefinition.new(*args) end def can_perform_case_insensitive_comparison_for?(column) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb index fe1dcbd710..70c0d28830 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb @@ -3,6 +3,13 @@ module ActiveRecord module SQLite3 class SchemaCreation < AbstractAdapter::SchemaCreation private + + def column_options(o) + options = super + options[:null] = false if o.primary_key + options + end + def add_column_options!(sql, options) if options[:collation] sql << " COLLATE \"#{options[:collation]}\"" diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 786b0ab2ed..4e5d9e65ec 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -85,15 +85,6 @@ module ActiveRecord @active = nil @statements = StatementPool.new(self.class.type_cast_config_to_integer(config.fetch(:statement_limit) { 1000 })) - - @visitor = Arel::Visitors::SQLite.new self - - if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) - @prepared_statements = true - @visitor.extend(DetermineIfPreparableVisitor) - else - @prepared_statements = false - end end def supports_ddl_transactions? diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 2ec9bf3d67..b8b8684cff 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -125,6 +125,10 @@ module ActiveRecord class InvalidForeignKey < WrappedDatabaseException end + # Raised when a record cannot be inserted or updated because a value too long for a column type. + class ValueTooLong < StatementInvalid + end + # Raised when number of bind variables in statement given to +:condition+ key # (for example, when using {ActiveRecord::Base.find}[rdoc-ref:FinderMethods#find] method) # does not match number of expected values supplied. diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index f2578f5f96..b397d1fd07 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -1,4 +1,3 @@ -require 'set' require 'active_support/concern' module ActiveRecord diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index 2c2d6cfa47..89396b518c 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -158,8 +158,9 @@ module ActiveRecord end end + ARRAY_WITH_EMPTY_STRING = [''] def non_empty_predicates - predicates - [''] + predicates - ARRAY_WITH_EMPTY_STRING end def wrap_sql_literal(node) diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index affcd9aed1..b4229cba04 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -138,6 +138,10 @@ HEADER table_options = @connection.table_options(table) tbl.print ", options: #{table_options.inspect}" unless table_options.blank? + if comment = @connection.table_comment(table) + tbl.print ", comment: #{comment.inspect}" + end + tbl.puts " do |t|" # then dump all non-primary key columns @@ -209,6 +213,7 @@ HEADER statement_parts << "where: #{index.where.inspect}" if index.where statement_parts << "using: #{index.using.inspect}" if index.using statement_parts << "type: #{index.type.inspect}" if index.type + statement_parts << "comment: #{index.comment.inspect}" if index.comment " #{statement_parts.join(', ')}" end diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 8881986f1b..9aea5b360b 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -107,8 +107,9 @@ module ActiveRecord def create(*arguments) configuration = arguments.first class_for_adapter(configuration['adapter']).new(*arguments).create + $stdout.puts "Created database '#{configuration['database']}'" rescue DatabaseAlreadyExists - $stderr.puts "#{configuration['database']} already exists" + $stderr.puts "Database '#{configuration['database']}' already exists" rescue Exception => error $stderr.puts error $stderr.puts "Couldn't create database for #{configuration.inspect}" @@ -133,11 +134,12 @@ module ActiveRecord def drop(*arguments) configuration = arguments.first class_for_adapter(configuration['adapter']).new(*arguments).drop + $stdout.puts "Dropped database '#{configuration['database']}'" rescue ActiveRecord::NoDatabaseError $stderr.puts "Database '#{configuration['database']}' does not exist" rescue Exception => error $stderr.puts error - $stderr.puts "Couldn't drop #{configuration['database']}" + $stderr.puts "Couldn't drop database '#{configuration['database']}'" raise end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 4a80cda0b8..1f59276137 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -67,9 +67,6 @@ module ActiveRecord cast_type = klass.type_for_attribute(attribute_name) value = cast_type.serialize(value) value = klass.connection.type_cast(value) - if value.is_a?(String) && column.limit - value = value.to_s[0, column.limit] - end comparison = if !options[:case_sensitive] && !value.nil? # will use SQL LOWER function before comparison, unless it detects a case insensitive collation diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 4f389e9249..32391e2e8b 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -2,6 +2,7 @@ require "cases/helper" require "models/book" require "models/post" require "models/author" +require "models/event" module ActiveRecord class AdapterTest < ActiveRecord::TestCase @@ -200,6 +201,14 @@ module ActiveRecord assert_not_nil error.cause end + + def test_value_limit_violations_are_translated_to_specific_exception + error = assert_raises(ActiveRecord::ValueTooLong) do + Event.create(title: 'abcdefgh') + end + + assert_not_nil error.cause + end end def test_disable_referential_integrity diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb new file mode 100644 index 0000000000..cb6f07c925 --- /dev/null +++ b/activerecord/test/cases/comment_test.rb @@ -0,0 +1,89 @@ +require 'cases/helper' +require 'support/schema_dumping_helper' + +class CommentTest < ActiveRecord::TestCase + include SchemaDumpingHelper + self.use_transactional_tests = false if current_adapter?(:Mysql2Adapter) + + class Commented < ActiveRecord::Base + self.table_name = 'commenteds' + end + + def setup + @connection = ActiveRecord::Base.connection + + @connection.transaction do + @connection.create_table('commenteds', comment: 'A table with comment', force: true) do |t| + t.string 'name', comment: 'Comment should help clarify the column purpose' + t.boolean 'obvious', comment: 'Question is: should you comment obviously named objects?' + t.string 'content' + t.index 'name', comment: %Q["Very important" index that powers all the performance.\nAnd it's fun!] + end + end + end + + teardown do + @connection.drop_table 'commenteds', if_exists: true + end + + if ActiveRecord::Base.connection.supports_comments? + def test_column_created_in_block + Commented.reset_column_information + column = Commented.columns_hash['name'] + assert_equal :string, column.type + assert_equal 'Comment should help clarify the column purpose', column.comment + end + + def test_add_column_with_comment_later + @connection.add_column :commenteds, :rating, :integer, comment: 'I am running out of imagination' + Commented.reset_column_information + column = Commented.columns_hash['rating'] + + assert_equal :integer, column.type + assert_equal 'I am running out of imagination', column.comment + end + + def test_add_index_with_comment_later + @connection.add_index :commenteds, :obvious, name: 'idx_obvious', comment: 'We need to see obvious comments' + index = @connection.indexes('commenteds').find { |idef| idef.name == 'idx_obvious' } + assert_equal 'We need to see obvious comments', index.comment + end + + def test_add_comment_to_column + @connection.change_column :commenteds, :content, :string, comment: 'Whoa, content describes itself!' + + Commented.reset_column_information + column = Commented.columns_hash['content'] + + assert_equal :string, column.type + assert_equal 'Whoa, content describes itself!', column.comment + end + + def test_remove_comment_from_column + @connection.change_column :commenteds, :obvious, :string, comment: nil + + Commented.reset_column_information + column = Commented.columns_hash['obvious'] + + assert_equal :string, column.type + assert_nil column.comment + end + + def test_schema_dump_with_comments + # Do all the stuff from other tests + @connection.add_column :commenteds, :rating, :integer, comment: 'I am running out of imagination' + @connection.change_column :commenteds, :content, :string, comment: 'Whoa, content describes itself!' + @connection.change_column :commenteds, :obvious, :string, comment: nil + @connection.add_index :commenteds, :obvious, name: 'idx_obvious', comment: 'We need to see obvious comments' + # And check that these changes are reflected in dump + output = dump_table_schema 'commenteds' + assert_match %r[create_table "commenteds",.+\s+comment: "A table with comment"], output + assert_match %r[t\.string\s+"name",\s+comment: "Comment should help clarify the column purpose"], output + assert_match %r[t\.string\s+"obvious"\n], output + assert_match %r[t\.string\s+"content",\s+comment: "Whoa, content describes itself!"], output + assert_match %r[t\.integer\s+"rating",\s+comment: "I am running out of imagination"], output + assert_match %r[add_index\s+.+\s+comment: "\\\"Very important\\\" index that powers all the performance.\\nAnd it's fun!"], output + assert_match %r[add_index\s+.+\s+name: "idx_obvious",.+\s+comment: "We need to see obvious comments"], output + end + end +end diff --git a/activerecord/test/cases/migration/references_foreign_key_test.rb b/activerecord/test/cases/migration/references_foreign_key_test.rb index 85435f4dbc..9e19eb9f73 100644 --- a/activerecord/test/cases/migration/references_foreign_key_test.rb +++ b/activerecord/test/cases/migration/references_foreign_key_test.rb @@ -145,6 +145,36 @@ module ActiveRecord end end + class CreateDogsMigration < ActiveRecord::Migration::Current + def change + create_table :dog_owners + + create_table :dogs do |t| + t.references :dog_owner, foreign_key: true + end + end + end + + def test_references_foreign_key_with_prefix + ActiveRecord::Base.table_name_prefix = 'p_' + migration = CreateDogsMigration.new + silence_stream($stdout) { migration.migrate(:up) } + assert_equal 1, @connection.foreign_keys("p_dogs").size + ensure + silence_stream($stdout) { migration.migrate(:down) } + ActiveRecord::Base.table_name_prefix = nil + end + + def test_references_foreign_key_with_suffix + ActiveRecord::Base.table_name_suffix = '_s' + migration = CreateDogsMigration.new + silence_stream($stdout) { migration.migrate(:up) } + assert_equal 1, @connection.foreign_keys("dogs_s").size + ensure + silence_stream($stdout) { migration.migrate(:down) } + ActiveRecord::Base.table_name_suffix = nil + end + test "multiple foreign keys can be added to the same table" do @connection.create_table :testings do |t| t.integer :col_1 diff --git a/activerecord/test/cases/migration/references_statements_test.rb b/activerecord/test/cases/migration/references_statements_test.rb index b9ce6bbc55..70c64f3e71 100644 --- a/activerecord/test/cases/migration/references_statements_test.rb +++ b/activerecord/test/cases/migration/references_statements_test.rb @@ -55,6 +55,11 @@ module ActiveRecord assert index_exists?(table_name, :tag_id, name: 'index_taggings_on_tag_id') end + def test_creates_named_unique_index + add_reference table_name, :tag, index: { name: 'index_taggings_on_tag_id', unique: true } + assert index_exists?(table_name, :tag_id, name: 'index_taggings_on_tag_id', unique: true ) + end + def test_creates_reference_id_with_specified_type add_reference table_name, :user, type: :string assert column_exists?(table_name, :user_id, :string) diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 32bccce2ed..52eac4a124 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -229,7 +229,7 @@ class PrimaryKeyAnyTypeTest < ActiveRecord::TestCase assert_equal "code", Barcode.primary_key column = Barcode.column_for_attribute(Barcode.primary_key) - assert_not column.null unless current_adapter?(:SQLite3Adapter) + assert_not column.null assert_equal :string, column.type assert_equal 42, column.limit end diff --git a/activerecord/test/cases/relation/record_fetch_warning_test.rb b/activerecord/test/cases/relation/record_fetch_warning_test.rb index 53daf436e5..0e0e23b24b 100644 --- a/activerecord/test/cases/relation/record_fetch_warning_test.rb +++ b/activerecord/test/cases/relation/record_fetch_warning_test.rb @@ -1,28 +1,40 @@ require 'cases/helper' require 'models/post' +require 'active_record/relation/record_fetch_warning' module ActiveRecord class RecordFetchWarningTest < ActiveRecord::TestCase fixtures :posts - def test_warn_on_records_fetched_greater_than - original_logger = ActiveRecord::Base.logger - original_warn_on_records_fetched_greater_than = ActiveRecord::Base.warn_on_records_fetched_greater_than + def setup + @original_logger = ActiveRecord::Base.logger + @original_warn_on_records_fetched_greater_than = ActiveRecord::Base.warn_on_records_fetched_greater_than + @log = StringIO.new + end + + def teardown + ActiveRecord::Base.logger = @original_logger + ActiveRecord::Base.warn_on_records_fetched_greater_than = @original_warn_on_records_fetched_greater_than + end - log = StringIO.new - ActiveRecord::Base.logger = ActiveSupport::Logger.new(log) + def test_warn_on_records_fetched_greater_than_allowed_limit + ActiveRecord::Base.logger = ActiveSupport::Logger.new(@log) ActiveRecord::Base.logger.level = Logger::WARN + ActiveRecord::Base.warn_on_records_fetched_greater_than = 1 - require 'active_record/relation/record_fetch_warning' + Post.all.to_a - ActiveRecord::Base.warn_on_records_fetched_greater_than = 1 + assert_match(/Query fetched/, @log.string) + end + + def test_does_not_warn_on_records_fetched_less_than_allowed_limit + ActiveRecord::Base.logger = ActiveSupport::Logger.new(@log) + ActiveRecord::Base.logger.level = Logger::WARN + ActiveRecord::Base.warn_on_records_fetched_greater_than = 100 Post.all.to_a - assert_match(/Query fetched/, log.string) - ensure - ActiveRecord::Base.logger = original_logger - ActiveRecord::Base.warn_on_records_fetched_greater_than = original_warn_on_records_fetched_greater_than + assert_no_match(/Query fetched/, @log.string) end end end diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 12c8a1d5ba..4498c96029 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -323,9 +323,9 @@ class SchemaDumperTest < ActiveRecord::TestCase create_table("dogs") do |t| t.column :name, :string t.column :owner_id, :integer + t.index [:name] + t.foreign_key :dog_owners, column: "owner_id" if supports_foreign_keys? end - add_index "dogs", [:name] - add_foreign_key :dogs, :dog_owners, column: "owner_id" if supports_foreign_keys? end def down drop_table("dogs") diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index 0aac5bad31..e6d731e1e1 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -8,6 +8,13 @@ module ActiveRecord ActiveRecord::Tasks::MySQLDatabaseTasks.stubs(:new).returns @mysql_tasks ActiveRecord::Tasks::PostgreSQLDatabaseTasks.stubs(:new).returns @postgresql_tasks ActiveRecord::Tasks::SQLiteDatabaseTasks.stubs(:new).returns @sqlite_tasks + + $stdout, @original_stdout = StringIO.new, $stdout + $stderr, @original_stderr = StringIO.new, $stderr + end + + def teardown + $stdout, $stderr = @original_stdout, @original_stderr end end diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index 1632f04854..8e480bbaee 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -1,4 +1,5 @@ require 'cases/helper' +require 'active_record/tasks/database_tasks' if current_adapter?(:Mysql2Adapter) module ActiveRecord @@ -12,6 +13,13 @@ module ActiveRecord ActiveRecord::Base.stubs(:connection).returns(@connection) ActiveRecord::Base.stubs(:establish_connection).returns(true) + + $stdout, @original_stdout = StringIO.new, $stdout + $stderr, @original_stderr = StringIO.new, $stderr + end + + def teardown + $stdout, $stderr = @original_stdout, @original_stderr end def test_establishes_connection_without_database @@ -48,14 +56,20 @@ module ActiveRecord ActiveRecord::Tasks::DatabaseTasks.create @configuration end - def test_create_when_database_exists_outputs_info_to_stderr - $stderr.expects(:puts).with("my-app-db already exists").once + def test_when_database_created_successfully_outputs_info_to_stdout + ActiveRecord::Tasks::DatabaseTasks.create @configuration + assert_equal $stdout.string, "Created database 'my-app-db'\n" + end + + def test_create_when_database_exists_outputs_info_to_stderr ActiveRecord::Base.connection.stubs(:create_database).raises( - ActiveRecord::StatementInvalid.new("Can't create database 'dev'; database exists:") + ActiveRecord::Tasks::DatabaseAlreadyExists ) ActiveRecord::Tasks::DatabaseTasks.create @configuration + + assert_equal $stderr.string, "Database 'my-app-db' already exists\n" end end @@ -77,6 +91,13 @@ module ActiveRecord ActiveRecord::Base.stubs(:establish_connection). raises(@error). then.returns(true) + + $stdout, @original_stdout = StringIO.new, $stdout + $stderr, @original_stderr = StringIO.new, $stderr + end + + def teardown + $stdout, $stderr = @original_stdout, @original_stderr end def test_root_password_is_requested @@ -160,6 +181,13 @@ module ActiveRecord ActiveRecord::Base.stubs(:connection).returns(@connection) ActiveRecord::Base.stubs(:establish_connection).returns(true) + + $stdout, @original_stdout = StringIO.new, $stdout + $stderr, @original_stderr = StringIO.new, $stderr + end + + def teardown + $stdout, $stderr = @original_stdout, @original_stderr end def test_establishes_connection_to_mysql_database @@ -173,6 +201,12 @@ module ActiveRecord ActiveRecord::Tasks::DatabaseTasks.drop @configuration end + + def test_when_database_dropped_successfully_outputs_info_to_stdout + ActiveRecord::Tasks::DatabaseTasks.drop @configuration + + assert_equal $stdout.string, "Dropped database 'my-app-db'\n" + end end class MySQLPurgeTest < ActiveRecord::TestCase @@ -307,6 +341,5 @@ module ActiveRecord ActiveRecord::Tasks::DatabaseTasks.structure_load(@configuration, filename) end end - end end diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb index ba53f340ae..6a0c7fbcb5 100644 --- a/activerecord/test/cases/tasks/postgresql_rake_test.rb +++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb @@ -1,4 +1,5 @@ require 'cases/helper' +require 'active_record/tasks/database_tasks' if current_adapter?(:PostgreSQLAdapter) module ActiveRecord @@ -12,6 +13,13 @@ module ActiveRecord ActiveRecord::Base.stubs(:connection).returns(@connection) ActiveRecord::Base.stubs(:establish_connection).returns(true) + + $stdout, @original_stdout = StringIO.new, $stdout + $stderr, @original_stderr = StringIO.new, $stderr + end + + def teardown + $stdout, $stderr = @original_stdout, @original_stderr end def test_establishes_connection_to_postgresql_database @@ -63,14 +71,20 @@ module ActiveRecord assert_raises(Exception) { ActiveRecord::Tasks::DatabaseTasks.create @configuration } end - def test_create_when_database_exists_outputs_info_to_stderr - $stderr.expects(:puts).with("my-app-db already exists").once + def test_when_database_created_successfully_outputs_info_to_stdout + ActiveRecord::Tasks::DatabaseTasks.create @configuration + assert_equal $stdout.string, "Created database 'my-app-db'\n" + end + + def test_create_when_database_exists_outputs_info_to_stderr ActiveRecord::Base.connection.stubs(:create_database).raises( - ActiveRecord::StatementInvalid.new('database "my-app-db" already exists') + ActiveRecord::Tasks::DatabaseAlreadyExists ) ActiveRecord::Tasks::DatabaseTasks.create @configuration + + assert_equal $stderr.string, "Database 'my-app-db' already exists\n" end end @@ -84,6 +98,13 @@ module ActiveRecord ActiveRecord::Base.stubs(:connection).returns(@connection) ActiveRecord::Base.stubs(:establish_connection).returns(true) + + $stdout, @original_stdout = StringIO.new, $stdout + $stderr, @original_stderr = StringIO.new, $stderr + end + + def teardown + $stdout, $stderr = @original_stdout, @original_stderr end def test_establishes_connection_to_postgresql_database @@ -101,6 +122,12 @@ module ActiveRecord ActiveRecord::Tasks::DatabaseTasks.drop @configuration end + + def test_when_database_dropped_successfully_outputs_info_to_stdout + ActiveRecord::Tasks::DatabaseTasks.drop @configuration + + assert_equal $stdout.string, "Dropped database 'my-app-db'\n" + end end class PostgreSQLPurgeTest < ActiveRecord::TestCase @@ -273,6 +300,5 @@ module ActiveRecord ActiveRecord::Tasks::DatabaseTasks.structure_load(@configuration, filename) end end - end end diff --git a/activerecord/test/cases/tasks/sqlite_rake_test.rb b/activerecord/test/cases/tasks/sqlite_rake_test.rb index 0aea0c3b38..4be03c7f61 100644 --- a/activerecord/test/cases/tasks/sqlite_rake_test.rb +++ b/activerecord/test/cases/tasks/sqlite_rake_test.rb @@ -1,4 +1,5 @@ require 'cases/helper' +require 'active_record/tasks/database_tasks' require 'pathname' if current_adapter?(:SQLite3Adapter) @@ -15,6 +16,13 @@ module ActiveRecord File.stubs(:exist?).returns(false) ActiveRecord::Base.stubs(:connection).returns(@connection) ActiveRecord::Base.stubs(:establish_connection).returns(true) + + $stdout, @original_stdout = StringIO.new, $stdout + $stderr, @original_stderr = StringIO.new, $stderr + end + + def teardown + $stdout, $stderr = @original_stdout, @original_stderr end def test_db_checks_database_exists @@ -23,12 +31,18 @@ module ActiveRecord ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root' end + def test_when_db_created_successfully_outputs_info_to_stdout + ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root' + + assert_equal $stdout.string, "Created database '#{@database}'\n" + end + def test_db_create_when_file_exists File.stubs(:exist?).returns(true) - $stderr.expects(:puts).with("#{@database} already exists") - ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root' + + assert_equal $stderr.string, "Database '#{@database}' already exists\n" end def test_db_create_with_file_does_nothing @@ -69,6 +83,13 @@ module ActiveRecord Pathname.stubs(:new).returns(@path) File.stubs(:join).returns('/former/relative/path') FileUtils.stubs(:rm).returns(true) + + $stdout, @original_stdout = StringIO.new, $stdout + $stderr, @original_stderr = StringIO.new, $stderr + end + + def teardown + $stdout, $stderr = @original_stdout, @original_stderr end def test_creates_path_from_database @@ -103,6 +124,12 @@ module ActiveRecord ActiveRecord::Tasks::DatabaseTasks.drop @configuration, '/rails/root' end + + def test_when_db_dropped_successfully_outputs_info_to_stdout + ActiveRecord::Tasks::DatabaseTasks.drop @configuration, '/rails/root' + + assert_equal $stdout.string, "Dropped database '#{@database}'\n" + end end class SqliteDBCharsetTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index 4c14d93c66..4b0a590adb 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -349,19 +349,41 @@ class UniquenessValidationTest < ActiveRecord::TestCase end def test_validate_uniqueness_with_limit - # Event.title is limited to 5 characters - e1 = Event.create(:title => "abcde") - assert e1.valid?, "Could not create an event with a unique, 5 character title" - e2 = Event.create(:title => "abcdefgh") - assert !e2.valid?, "Created an event whose title, with limit taken into account, is not unique" + if current_adapter?(:SQLite3Adapter) + # Event.title has limit 5, but SQLite doesn't truncate. + e1 = Event.create(title: "abcdefgh") + assert e1.valid?, "Could not create an event with a unique 8 characters title" + + e2 = Event.create(title: "abcdefgh") + assert_not e2.valid?, "Created an event whose title is not unique" + elsif current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter, :SQLServerAdapter) + assert_raise(ActiveRecord::ValueTooLong) do + Event.create(title: "abcdefgh") + end + else + assert_raise(ActiveRecord::StatementInvalid) do + Event.create(title: "abcdefgh") + end + end end def test_validate_uniqueness_with_limit_and_utf8 - # Event.title is limited to 5 characters - e1 = Event.create(:title => "一二三四五") - assert e1.valid?, "Could not create an event with a unique, 5 character title" - e2 = Event.create(:title => "一二三四五六七八") - assert !e2.valid?, "Created an event whose title, with limit taken into account, is not unique" + if current_adapter?(:SQLite3Adapter) + # Event.title has limit 5, but does SQLite doesn't truncate. + e1 = Event.create(title: "一二三四五六七八") + assert e1.valid?, "Could not create an event with a unique 8 characters title" + + e2 = Event.create(title: "一二三四五六七八") + assert_not e2.valid?, "Created an event whose title is not unique" + elsif current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter, :SQLServerAdapter) + assert_raise(ActiveRecord::ValueTooLong) do + Event.create(title: "一二三四五六七八") + end + else + assert_raise(ActiveRecord::StatementInvalid) do + Event.create(title: "一二三四五六七八") + end + end end def test_validate_straight_inheritance_uniqueness diff --git a/activerecord/test/schema/sqlite_specific_schema.rb b/activerecord/test/schema/sqlite_specific_schema.rb index b5552c2755..cc7c36fe2b 100644 --- a/activerecord/test/schema/sqlite_specific_schema.rb +++ b/activerecord/test/schema/sqlite_specific_schema.rb @@ -1,8 +1,4 @@ ActiveRecord::Schema.define do - create_table :table_with_autoincrement, :force => true do |t| - t.column :name, :string - end - execute "DROP TABLE fk_test_has_fk" rescue nil execute "DROP TABLE fk_test_has_pk" rescue nil execute <<_SQL diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 5729a98b99..21dd0657aa 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,70 @@ +* `Array#sum` compat with Ruby 2.4's native method. + + Ruby 2.4 introduces `Array#sum`, but it only supports numeric elements, + breaking our `Enumerable#sum` which supports arbitrary `Object#+`. + To fix, override `Array#sum` with our compatible implementation. + + Native Ruby 2.4: + + %w[ a b ].sum + # => TypeError: String can't be coerced into Fixnum + + With `Enumerable#sum` shim: + + %w[ a b ].sum + # => 'ab' + + We tried shimming the fast path and falling back to the compatible path + if it fails, but that ends up slower even in simple cases due to the cost + of exception handling. Our only choice is to override the native `Array#sum` + with our `Enumerable#sum`. + + *Jeremy Daer* + +* `ActiveSupport::Duration` supports ISO8601 formatting and parsing. + + ActiveSupport::Duration.parse('P3Y6M4DT12H30M5S') + # => 3 years, 6 months, 4 days, 12 hours, 30 minutes, and 5 seconds + + (3.years + 3.days).iso8601 + # => "P3Y3D" + + Inspired by Arnau Siches' [ISO8601 gem](https://github.com/arnau/ISO8601/) + and rewritten by Andrey Novikov with suggestions from Andrew White. Test + data from the ISO8601 gem redistributed under MIT license. + + (Will be used to support the PostgreSQL interval data type.) + + *Andrey Novikov*, *Arnau Siches*, *Andrew White* + +* `Cache#fetch(key, force: true)` forces a cache miss, so it must be called + with a block to provide a new value to cache. Fetching with `force: true` + but without a block now raises ArgumentError. + + cache.fetch('key', force: true) # => ArgumentError + + *Santosh Wadghule* + +* `ActiveSupport::Duration` supports weeks and hours. + + [1.hour.inspect, 1.hour.value, 1.hour.parts] + # => ["3600 seconds", 3600, [[:seconds, 3600]]] # Before + # => ["1 hour", 3600, [[:hours, 1]]] # After + + [1.week.inspect, 1.week.value, 1.week.parts] + # => ["7 days", 604800, [[:days, 7]]] # Before + # => ["1 week", 604800, [[:weeks, 1]]] # After + + This brings us into closer conformance with ISO8601 and relieves some + astonishment about getting `1.hour.inspect # => 3600 seconds`. + + Compatibility: The duration's `value` remains the same, so apps using + durations are oblivious to the new time periods. Apps, libraries, and + plugins that work with the internal `parts` hash will need to broaden + their time period handling to cover hours & weeks. + + *Andrey Novikov* + * Fix behavior of JSON encoding for `Exception`. *namusyaka* diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 1c63e8a93f..bc114e0785 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -158,20 +158,20 @@ module ActiveSupport attr_reader :silence, :options alias :silence? :silence - # Create a new cache. The options will be passed to any write method calls + # Creates a new cache. The options will be passed to any write method calls # except for <tt>:namespace</tt> which can be used to set the global # namespace for the cache. def initialize(options = nil) @options = options ? options.dup : {} end - # Silence the logger. + # Silences the logger. def silence! @silence = true self end - # Silence the logger within a block. + # Silences the logger within a block. def mute previous_silence, @silence = defined?(@silence) && @silence, true yield @@ -198,10 +198,17 @@ module ActiveSupport # cache.fetch('city') # => "Duckburgh" # # You may also specify additional options via the +options+ argument. - # Setting <tt>force: true</tt> will force a cache miss: + # Setting <tt>force: true</tt> forces a cache "miss," meaning we treat + # the cache value as missing even if it's present. Passing a block is + # required when `force` is true so this always results in a cache write. # # cache.write('today', 'Monday') - # cache.fetch('today', force: true) # => nil + # cache.fetch('today', force: true) { 'Tuesday' } # => 'Tuesday' + # cache.fetch('today', force: true) # => ArgumentError + # + # The `:force` option is useful when you're calling some other method to + # ask whether you should force a cache write. Otherwise, it's clearer to + # just call `Cache#write`. # # Setting <tt>:compress</tt> will store a large cache entry set by the call # in a compressed format. @@ -292,6 +299,8 @@ module ActiveSupport else save_block_result_to_cache(name, options) { |_name| yield _name } end + elsif options && options[:force] + raise ArgumentError, 'Missing block: Calling `Cache#fetch` with `force: true` requires a block.' else read(name, options) end @@ -323,7 +332,7 @@ module ActiveSupport end end - # Read multiple values at once from the cache. Options can be passed + # Reads multiple values at once from the cache. Options can be passed # in the last argument. # # Some cache implementation may optimize this method. @@ -413,7 +422,7 @@ module ActiveSupport end end - # Delete all entries with keys matching the pattern. + # Deletes all entries with keys matching the pattern. # # Options are passed to the underlying cache implementation. # @@ -422,7 +431,7 @@ module ActiveSupport raise NotImplementedError.new("#{self.class.name} does not support delete_matched") end - # Increment an integer value in the cache. + # Increments an integer value in the cache. # # Options are passed to the underlying cache implementation. # @@ -431,7 +440,7 @@ module ActiveSupport raise NotImplementedError.new("#{self.class.name} does not support increment") end - # Decrement an integer value in the cache. + # Decrements an integer value in the cache. # # Options are passed to the underlying cache implementation. # @@ -440,7 +449,7 @@ module ActiveSupport raise NotImplementedError.new("#{self.class.name} does not support decrement") end - # Cleanup the cache by removing expired entries. + # Cleanups the cache by removing expired entries. # # Options are passed to the underlying cache implementation. # @@ -449,7 +458,7 @@ module ActiveSupport raise NotImplementedError.new("#{self.class.name} does not support cleanup") end - # Clear the entire cache. Be careful with this method since it could + # Clears the entire cache. Be careful with this method since it could # affect other processes if shared cache is being used. # # The options hash is passed to the underlying cache implementation. @@ -460,7 +469,7 @@ module ActiveSupport end protected - # Add the namespace defined in the options to a pattern designed to + # Adds the namespace defined in the options to a pattern designed to # match keys. Implementations that support delete_matched should call # this method to translate a pattern that matches names into one that # matches namespaced keys. @@ -479,26 +488,26 @@ module ActiveSupport end end - # Read an entry from the cache implementation. Subclasses must implement + # Reads an entry from the cache implementation. Subclasses must implement # this method. def read_entry(key, options) # :nodoc: raise NotImplementedError.new end - # Write an entry to the cache implementation. Subclasses must implement + # Writes an entry to the cache implementation. Subclasses must implement # this method. def write_entry(key, entry, options) # :nodoc: raise NotImplementedError.new end - # Delete an entry from the cache implementation. Subclasses must + # Deletes an entry from the cache implementation. Subclasses must # implement this method. def delete_entry(key, options) # :nodoc: raise NotImplementedError.new end private - # Merge the default options with ones specific to a method call. + # Merges the default options with ones specific to a method call. def merged_options(call_options) # :nodoc: if call_options options.merge(call_options) @@ -507,7 +516,7 @@ module ActiveSupport end end - # Expand key to be a consistent string value. Invoke +cache_key+ if + # Expands key to be a consistent string value. Invokes +cache_key+ if # object responds to +cache_key+. Otherwise, +to_param+ method will be # called. If the key is a Hash, then keys will be sorted alphabetically. def expanded_key(key) # :nodoc: @@ -527,7 +536,7 @@ module ActiveSupport key.to_param end - # Prefix a key with the namespace. Namespace and key will be delimited + # Prefixes a key with the namespace. Namespace and key will be delimited # with a colon. def normalize_key(key, options) key = expanded_key(key) @@ -575,12 +584,12 @@ module ActiveSupport end def get_entry_value(entry, name, options) - instrument(:fetch_hit, name, options) { |payload| } + instrument(:fetch_hit, name, options) { } entry.value end def save_block_result_to_cache(name, options) - result = instrument(:generate, name, options) do |payload| + result = instrument(:generate, name, options) do yield(name) end @@ -598,7 +607,7 @@ module ActiveSupport class Entry # :nodoc: DEFAULT_COMPRESS_LIMIT = 16.kilobytes - # Create a new cache entry for the specified value. Options supported are + # Creates a new cache entry for the specified value. Options supported are # +:compress+, +:compress_threshold+, and +:expires_in+. def initialize(value, options = {}) if should_compress?(value, options) @@ -617,7 +626,7 @@ module ActiveSupport compressed? ? uncompress(@value) : @value end - # Check if the entry is expired. The +expires_in+ parameter can override + # Checks if the entry is expired. The +expires_in+ parameter can override # the value set when the entry was created. def expired? @expires_in && @created_at + @expires_in <= Time.now.to_f @@ -652,7 +661,7 @@ module ActiveSupport end end - # Duplicate the value in a class. This is used by cache implementations that don't natively + # Duplicates the value in a class. This is used by cache implementations that don't natively # serialize entries to protect against accidental cache modifications. def dup_value! if @value && !compressed? && !(@value.is_a?(Numeric) || @value == true || @value == false) diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index d878d44d02..904d3f0eb0 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -782,7 +782,7 @@ module ActiveSupport def display_deprecation_warning_for_false_terminator ActiveSupport::Deprecation.warn(<<-MSG.squish) - Returning `false` in Active Record and Active Model callbacks will not implicitly halt a callback chain in the next release of Rails. + Returning `false` in Active Record and Active Model callbacks will not implicitly halt a callback chain in Rails 5.1. To explicitly halt the callback chain, please use `throw :abort` instead. MSG end diff --git a/activesupport/lib/active_support/core_ext/date/conversions.rb b/activesupport/lib/active_support/core_ext/date/conversions.rb index ed8bca77ac..9a6d7bb415 100644 --- a/activesupport/lib/active_support/core_ext/date/conversions.rb +++ b/activesupport/lib/active_support/core_ext/date/conversions.rb @@ -80,6 +80,7 @@ class Date # # date.to_time(:utc) # => 2007-11-10 00:00:00 UTC def to_time(form = :local) + raise ArgumentError, "Expected :local or :utc, got #{form.inspect}." unless [:local, :utc].include?(form) ::Time.send(form, year, month, day) end diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index 8a74ad4d66..0e03f7d7be 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -104,3 +104,17 @@ class Range #:nodoc: end end end + +# Array#sum was added in Ruby 2.4 but it only works with Numeric elements. +# +# We tried shimming it to attempt the fast native method, rescue TypeError, +# and fall back to the compatible implementation, but that's much slower than +# just calling the compat method in the first place. +if Array.instance_methods(false).include?(:sum) && !(%w[a].sum rescue false) + class Array + def sum(*args) #:nodoc: + # Use Enumerable#sum instead. + super + end + end +end diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index 6741e732f0..dd5ebe6d8d 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -31,7 +31,7 @@ class Hash # with +key+ as <tt>:root</tt>, and +key+ singularized as second argument. The # callable can add nodes by using <tt>options[:builder]</tt>. # - # 'foo'.to_xml(lambda { |options, key| options[:builder].b(key) }) + # {foo: lambda { |options, key| options[:builder].b(key) }}.to_xml # # => "<b>foo</b>" # # * If +value+ responds to +to_xml+ the method is invoked with +key+ as <tt>:root</tt>. diff --git a/activesupport/lib/active_support/core_ext/numeric/time.rb b/activesupport/lib/active_support/core_ext/numeric/time.rb index 6c4a975495..c6ece22f8d 100644 --- a/activesupport/lib/active_support/core_ext/numeric/time.rb +++ b/activesupport/lib/active_support/core_ext/numeric/time.rb @@ -25,17 +25,17 @@ class Numeric # Returns a Duration instance matching the number of minutes provided. # - # 2.minutes # => 120 seconds + # 2.minutes # => 2 minutes def minutes - ActiveSupport::Duration.new(self * 60, [[:seconds, self * 60]]) + ActiveSupport::Duration.new(self * 60, [[:minutes, self]]) end alias :minute :minutes # Returns a Duration instance matching the number of hours provided. # - # 2.hours # => 7_200 seconds + # 2.hours # => 2 hours def hours - ActiveSupport::Duration.new(self * 3600, [[:seconds, self * 3600]]) + ActiveSupport::Duration.new(self * 3600, [[:hours, self]]) end alias :hour :hours @@ -49,17 +49,17 @@ class Numeric # Returns a Duration instance matching the number of weeks provided. # - # 2.weeks # => 14 days + # 2.weeks # => 2 weeks def weeks - ActiveSupport::Duration.new(self * 7.days, [[:days, self * 7]]) + ActiveSupport::Duration.new(self * 7.days, [[:weeks, self]]) end alias :week :weeks # Returns a Duration instance matching the number of fortnights provided. # - # 2.fortnights # => 28 days + # 2.fortnights # => 4 weeks def fortnights - ActiveSupport::Duration.new(self * 2.weeks, [[:days, self * 14]]) + ActiveSupport::Duration.new(self * 2.weeks, [[:weeks, self * 2]]) end alias :fortnight :fortnights diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb index c63b61e97a..3bde541009 100644 --- a/activesupport/lib/active_support/duration.rb +++ b/activesupport/lib/active_support/duration.rb @@ -9,6 +9,9 @@ module ActiveSupport class Duration attr_accessor :value, :parts + autoload :ISO8601Parser, 'active_support/duration/iso8601_parser' + autoload :ISO8601Serializer, 'active_support/duration/iso8601_serializer' + def initialize(value, parts) #:nodoc: @value, @parts = value, parts end @@ -117,7 +120,7 @@ module ActiveSupport def inspect #:nodoc: parts. reduce(::Hash.new(0)) { |h,(l,r)| h[l] += r; h }. - sort_by {|unit, _ | [:years, :months, :days, :minutes, :seconds].index(unit)}. + sort_by {|unit, _ | [:years, :months, :weeks, :days, :hours, :minutes, :seconds].index(unit)}. map {|unit, val| "#{val} #{val == 1 ? unit.to_s.chop : unit.to_s}"}. to_sentence(locale: ::I18n.default_locale) end @@ -130,6 +133,23 @@ module ActiveSupport @value.respond_to?(method, include_private) end + # Creates a new Duration from string formatted according to ISO 8601 Duration. + # + # See {ISO 8601}[http://en.wikipedia.org/wiki/ISO_8601#Durations] for more information. + # This method allows negative parts to be present in pattern. + # If invalid string is provided, it will raise +ActiveSupport::Duration::ISO8601Parser::ParsingError+. + def self.parse(iso8601duration) + parts = ISO8601Parser.new(iso8601duration).parse! + time = ::Time.current + new(time.advance(parts) - time, parts) + end + + # Build ISO 8601 Duration string for this duration. + # The +precision+ parameter can be used to limit seconds' precision of duration. + def iso8601(precision: nil) + ISO8601Serializer.new(self, precision: precision).serialize + end + delegate :<=>, to: :value protected @@ -139,6 +159,8 @@ module ActiveSupport if t.acts_like?(:time) || t.acts_like?(:date) if type == :seconds t.since(sign * number) + elsif [:hours, :minutes].include?(type) + t.in_time_zone.advance(type => sign * number) else t.advance(type => sign * number) end diff --git a/activesupport/lib/active_support/duration/iso8601_parser.rb b/activesupport/lib/active_support/duration/iso8601_parser.rb new file mode 100644 index 0000000000..07af58ad99 --- /dev/null +++ b/activesupport/lib/active_support/duration/iso8601_parser.rb @@ -0,0 +1,122 @@ +require 'strscan' + +module ActiveSupport + class Duration + # Parses a string formatted according to ISO 8601 Duration into the hash. + # + # See {ISO 8601}[http://en.wikipedia.org/wiki/ISO_8601#Durations] for more information. + # + # This parser allows negative parts to be present in pattern. + class ISO8601Parser # :nodoc: + class ParsingError < ::ArgumentError; end + + PERIOD_OR_COMMA = /\.|,/ + PERIOD = '.'.freeze + COMMA = ','.freeze + + SIGN_MARKER = /\A\-|\+|/ + DATE_MARKER = /P/ + TIME_MARKER = /T/ + DATE_COMPONENT = /(\-?\d+(?:[.,]\d+)?)(Y|M|D|W)/ + TIME_COMPONENT = /(\-?\d+(?:[.,]\d+)?)(H|M|S)/ + + DATE_TO_PART = { 'Y' => :years, 'M' => :months, 'W' => :weeks, 'D' => :days } + TIME_TO_PART = { 'H' => :hours, 'M' => :minutes, 'S' => :seconds } + + DATE_COMPONENTS = [:years, :months, :days] + TIME_COMPONENTS = [:hours, :minutes, :seconds] + + attr_reader :parts, :scanner + attr_accessor :mode, :sign + + def initialize(string) + @scanner = StringScanner.new(string) + @parts = {} + @mode = :start + @sign = 1 + end + + def parse! + while !finished? + case mode + when :start + if scan(SIGN_MARKER) + self.sign = (scanner.matched == '-') ? -1 : 1 + self.mode = :sign + else + raise_parsing_error + end + + when :sign + if scan(DATE_MARKER) + self.mode = :date + else + raise_parsing_error + end + + when :date + if scan(TIME_MARKER) + self.mode = :time + elsif scan(DATE_COMPONENT) + parts[DATE_TO_PART[scanner[2]]] = number * sign + else + raise_parsing_error + end + + when :time + if scan(TIME_COMPONENT) + parts[TIME_TO_PART[scanner[2]]] = number * sign + else + raise_parsing_error + end + + end + end + + validate! + parts + end + + private + + def finished? + scanner.eos? + end + + # Parses number which can be a float with either comma or period. + def number + scanner[1] =~ PERIOD_OR_COMMA ? scanner[1].tr(COMMA, PERIOD).to_f : scanner[1].to_i + end + + def scan(pattern) + scanner.scan(pattern) + end + + def raise_parsing_error(reason = nil) + raise ParsingError, "Invalid ISO 8601 duration: #{scanner.string.inspect} #{reason}".strip + end + + # Checks for various semantic errors as stated in ISO 8601 standard. + def validate! + raise_parsing_error('is empty duration') if parts.empty? + + # Mixing any of Y, M, D with W is invalid. + if parts.key?(:weeks) && (parts.keys & DATE_COMPONENTS).any? + raise_parsing_error('mixing weeks with other date parts not allowed') + end + + # Specifying an empty T part is invalid. + if mode == :time && (parts.keys & TIME_COMPONENTS).empty? + raise_parsing_error('time part marker is present but time part is empty') + end + + fractions = parts.values.reject(&:zero?).select { |a| (a % 1) != 0 } + unless fractions.empty? || (fractions.size == 1 && fractions.last == @parts.values.reject(&:zero?).last) + raise_parsing_error '(only last part can be fractional)' + end + + return true + end + end + end +end diff --git a/activesupport/lib/active_support/duration/iso8601_serializer.rb b/activesupport/lib/active_support/duration/iso8601_serializer.rb new file mode 100644 index 0000000000..05c6a083a9 --- /dev/null +++ b/activesupport/lib/active_support/duration/iso8601_serializer.rb @@ -0,0 +1,51 @@ +require 'active_support/core_ext/object/blank' +require 'active_support/core_ext/hash/transform_values' + +module ActiveSupport + class Duration + # Serializes duration to string according to ISO 8601 Duration format. + class ISO8601Serializer + def initialize(duration, precision: nil) + @duration = duration + @precision = precision + end + + # Builds and returns output string. + def serialize + output = 'P' + parts, sign = normalize + output << "#{parts[:years]}Y" if parts.key?(:years) + output << "#{parts[:months]}M" if parts.key?(:months) + output << "#{parts[:weeks]}W" if parts.key?(:weeks) + output << "#{parts[:days]}D" if parts.key?(:days) + time = '' + time << "#{parts[:hours]}H" if parts.key?(:hours) + time << "#{parts[:minutes]}M" if parts.key?(:minutes) + if parts.key?(:seconds) + time << "#{sprintf(@precision ? "%0.0#{@precision}f" : '%g', parts[:seconds])}S" + end + output << "T#{time}" if time.present? + "#{sign}#{output}" + end + + private + + # Return pair of duration's parts and whole duration sign. + # Parts are summarized (as they can become repetitive due to addition, etc). + # Zero parts are removed as not significant. + # If all parts are negative it will negate all of them and return minus as a sign. + def normalize + parts = @duration.parts.each_with_object(Hash.new(0)) do |(k,v),p| + p[k] += v unless v.zero? + end + # If all parts are negative - let's make a negative duration + sign = '' + if parts.values.all? { |v| v < 0 } + sign = '-' + parts.transform_values!(&:-@) + end + [parts, sign] + end + end + end +end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 9e744afb2b..ec7d028d7e 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -266,6 +266,20 @@ module CacheStoreBehavior end end + def test_fetch_with_forced_cache_miss_with_block + @cache.write('foo', 'bar') + assert_equal 'foo_bar', @cache.fetch('foo', force: true) { 'foo_bar' } + end + + def test_fetch_with_forced_cache_miss_without_block + @cache.write('foo', 'bar') + assert_raises(ArgumentError) do + @cache.fetch('foo', force: true) + end + + assert_equal 'bar', @cache.read('foo') + end + def test_should_read_and_write_hash assert @cache.write('foo', {:a => "b"}) assert_equal({:a => "b"}, @cache.read('foo')) diff --git a/activesupport/test/core_ext/date_ext_test.rb b/activesupport/test/core_ext/date_ext_test.rb index 0fc3f765f5..932675a50d 100644 --- a/activesupport/test/core_ext/date_ext_test.rb +++ b/activesupport/test/core_ext/date_ext_test.rb @@ -49,6 +49,10 @@ class DateExtCalculationsTest < ActiveSupport::TestCase end end end + + assert_raise(ArgumentError) do + Date.new(2005, 2, 21).to_time(:tokyo) + end end def test_compare_to_time diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 9e97acaffb..bef660fe12 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -66,8 +66,9 @@ class DurationTest < ActiveSupport::TestCase assert_equal '10 years, 2 months, and 1 day', (10.years + 2.months + 1.day).inspect assert_equal '10 years, 2 months, and 1 day', (10.years + 1.month + 1.day + 1.month).inspect assert_equal '10 years, 2 months, and 1 day', (1.day + 10.years + 2.months).inspect - assert_equal '7 days', 1.week.inspect - assert_equal '14 days', 1.fortnight.inspect + assert_equal '7 days', 7.days.inspect + assert_equal '1 week', 1.week.inspect + assert_equal '2 weeks', 1.fortnight.inspect end def test_inspect_locale @@ -222,4 +223,89 @@ class DurationTest < ActiveSupport::TestCase assert_equal(1, (1.minute <=> 1.second)) assert_equal(1, (61 <=> 1.minute)) end + + # ISO8601 string examples are taken from ISO8601 gem at https://github.com/arnau/ISO8601/blob/b93d466840/spec/iso8601/duration_spec.rb + # published under the conditions of MIT license at https://github.com/arnau/ISO8601/blob/b93d466840/LICENSE + # + # Copyright (c) 2012-2014 Arnau Siches + # + # MIT License + # + # Permission is hereby granted, free of charge, to any person obtaining + # a copy of this software and associated documentation files (the + # "Software"), to deal in the Software without restriction, including + # without limitation the rights to use, copy, modify, merge, publish, + # distribute, sublicense, and/or sell copies of the Software, and to + # permit persons to whom the Software is furnished to do so, subject to + # the following conditions: + # + # The above copyright notice and this permission notice shall be + # included in all copies or substantial portions of the Software. + # + # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + # EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + # MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + # NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE + # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION + # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + + def test_iso8601_parsing_wrong_patterns_with_raise + invalid_patterns = ['', 'P', 'PT', 'P1YT', 'T', 'PW', 'P1Y1W', '~P1Y', '.P1Y', 'P1.5Y0.5M', 'P1.5Y1M', 'P1.5MT10.5S'] + invalid_patterns.each do |pattern| + assert_raise ActiveSupport::Duration::ISO8601Parser::ParsingError, pattern.inspect do + ActiveSupport::Duration.parse(pattern) + end + end + end + + def test_iso8601_output + expectations = [ + ['P1Y', 1.year ], + ['P1W', 1.week ], + ['P1Y1M', 1.year + 1.month ], + ['P1Y1M1D', 1.year + 1.month + 1.day ], + ['-P1Y1D', -1.year - 1.day ], + ['P1Y-1DT-1S', 1.year - 1.day - 1.second ], # Parts with different signs are exists in PostgreSQL interval datatype. + ['PT1S', 1.second ], + ['PT1.4S', (1.4).seconds ], + ['P1Y1M1DT1H', 1.year + 1.month + 1.day + 1.hour], + ] + expectations.each do |expected_output, duration| + assert_equal expected_output, duration.iso8601, expected_output.inspect + end + end + + def test_iso8601_output_precision + expectations = [ + [nil, 'P1Y1MT5.55S', 1.year + 1.month + (5.55).seconds ], + [0, 'P1Y1MT6S', 1.year + 1.month + (5.55).seconds ], + [1, 'P1Y1MT5.5S', 1.year + 1.month + (5.55).seconds ], + [2, 'P1Y1MT5.55S', 1.year + 1.month + (5.55).seconds ], + [3, 'P1Y1MT5.550S', 1.year + 1.month + (5.55).seconds ], + [nil, 'PT1S', 1.second ], + [2, 'PT1.00S', 1.second ], + [nil, 'PT1.4S', (1.4).seconds ], + [0, 'PT1S', (1.4).seconds ], + [1, 'PT1.4S', (1.4).seconds ], + [5, 'PT1.40000S', (1.4).seconds ], + ] + expectations.each do |precision, expected_output, duration| + assert_equal expected_output, duration.iso8601(precision: precision), expected_output.inspect + end + end + + def test_iso8601_output_and_reparsing + patterns = %w[ + P1Y P0.5Y P0,5Y P1Y1M P1Y0.5M P1Y0,5M P1Y1M1D P1Y1M0.5D P1Y1M0,5D P1Y1M1DT1H P1Y1M1DT0.5H P1Y1M1DT0,5H P1W +P1Y -P1Y + P1Y1M1DT1H1M P1Y1M1DT1H0.5M P1Y1M1DT1H0,5M P1Y1M1DT1H1M1S P1Y1M1DT1H1M1.0S P1Y1M1DT1H1M1,0S P-1Y-2M3DT-4H-5M-6S + ] + # That could be weird, but if we parse P1Y1M0.5D and output it to ISO 8601, we'll get P1Y1MT12.0H. + # So we check that initially parsed and reparsed duration added to time will result in the same time. + time = Time.current + patterns.each do |pattern| + duration = ActiveSupport::Duration.parse(pattern) + assert_equal time+duration, time+ActiveSupport::Duration.parse(duration.iso8601), pattern.inspect + end + end end diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index f09b7d8850..976c8b2b81 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -10,22 +10,22 @@ class SummablePayment < Payment end class EnumerableTests < ActiveSupport::TestCase - class GenericEnumerable include Enumerable + def initialize(values = [1, 2, 3]) @values = values end def each - @values.each{|v| yield v} + @values.each { |v| yield v } end end def test_sums enum = GenericEnumerable.new([5, 15, 10]) assert_equal 30, enum.sum - assert_equal 60, enum.sum { |i| i * 2} + assert_equal 60, enum.sum { |i| i * 2 } enum = GenericEnumerable.new(%w(a b c)) assert_equal 'abc', enum.sum @@ -70,6 +70,24 @@ class EnumerableTests < ActiveSupport::TestCase assert_equal 42, (10...10).sum(42) end + def test_array_sums + enum = [5, 15, 10] + assert_equal 30, enum.sum + assert_equal 60, enum.sum { |i| i * 2 } + + enum = %w(a b c) + assert_equal 'abc', enum.sum + assert_equal 'aabbcc', enum.sum { |i| i * 2 } + + payments = [ Payment.new(5), Payment.new(15), Payment.new(10) ] + assert_equal 30, payments.sum(&:price) + assert_equal 60, payments.sum { |p| p.price * 2 } + + payments = [ SummablePayment.new(5), SummablePayment.new(15) ] + assert_equal SummablePayment.new(20), payments.sum + assert_equal SummablePayment.new(20), payments.sum { |p| p } + end + def test_index_by payments = GenericEnumerable.new([ Payment.new(5), Payment.new(15), Payment.new(10) ]) assert_equal({ 5 => Payment.new(5), 15 => Payment.new(15), 10 => Payment.new(10) }, diff --git a/activesupport/test/multibyte_conformance_test.rb b/activesupport/test/multibyte_conformance_test.rb index 5df8f32e46..9fca47a985 100644 --- a/activesupport/test/multibyte_conformance_test.rb +++ b/activesupport/test/multibyte_conformance_test.rb @@ -28,7 +28,7 @@ class MultibyteConformanceTest < ActiveSupport::TestCase UNIDATA_URL = "http://www.unicode.org/Public/#{ActiveSupport::Multibyte::Unicode::UNICODE_VERSION}/ucd" UNIDATA_FILE = '/NormalizationTest.txt' - CACHE_DIR = File.join(Dir.tmpdir, 'cache') + CACHE_DIR = "#{Dir.tmpdir}/cache/unicode_conformance" FileUtils.mkdir_p(CACHE_DIR) RUN_P = begin Downloader.download(UNIDATA_URL + UNIDATA_FILE, CACHE_DIR + UNIDATA_FILE) diff --git a/activesupport/test/multibyte_grapheme_break_conformance_test.rb b/activesupport/test/multibyte_grapheme_break_conformance_test.rb index 229f24990e..6e2f02abed 100644 --- a/activesupport/test/multibyte_grapheme_break_conformance_test.rb +++ b/activesupport/test/multibyte_grapheme_break_conformance_test.rb @@ -27,7 +27,7 @@ class MultibyteGraphemeBreakConformanceTest < ActiveSupport::TestCase TEST_DATA_URL = "http://www.unicode.org/Public/#{ActiveSupport::Multibyte::Unicode::UNICODE_VERSION}/ucd/auxiliary" TEST_DATA_FILE = '/GraphemeBreakTest.txt' - CACHE_DIR = File.join(Dir.tmpdir, 'cache') + CACHE_DIR = "#{Dir.tmpdir}/cache/unicode_conformance" def setup FileUtils.mkdir_p(CACHE_DIR) diff --git a/activesupport/test/multibyte_normalization_conformance_test.rb b/activesupport/test/multibyte_normalization_conformance_test.rb index 8bc91ef708..0d31c9520f 100644 --- a/activesupport/test/multibyte_normalization_conformance_test.rb +++ b/activesupport/test/multibyte_normalization_conformance_test.rb @@ -30,7 +30,7 @@ class MultibyteNormalizationConformanceTest < ActiveSupport::TestCase UNIDATA_URL = "http://www.unicode.org/Public/#{ActiveSupport::Multibyte::Unicode::UNICODE_VERSION}/ucd" UNIDATA_FILE = '/NormalizationTest.txt' - CACHE_DIR = File.join(Dir.tmpdir, 'cache') + CACHE_DIR = "#{Dir.tmpdir}/cache/unicode_conformance" def setup FileUtils.mkdir_p(CACHE_DIR) diff --git a/guides/source/5_0_release_notes.md b/guides/source/5_0_release_notes.md index 6351ff57c1..28f653b634 100644 --- a/guides/source/5_0_release_notes.md +++ b/guides/source/5_0_release_notes.md @@ -585,6 +585,9 @@ Please refer to the [Changelog][active-record] for detailed changes. * Added ActiveRecord `#second_to_last` and `#third_to_last` methods. ([Pull Request](https://github.com/rails/rails/pull/23583)) +* Added ability to annotate database objects (tables, columns, indexes) + with comments stored in database metadata for PostgreSQL & MySQL. + ([Pull Request](https://github.com/rails/rails/pull/22911)) Active Model ------------ @@ -821,6 +824,9 @@ Please refer to the [Changelog][active-support] for detailed changes. application code, and the application reloading process. ([Pull Request](https://github.com/rails/rails/pull/23807)) +* `ActiveSupport::Duration` now supports ISO8601 formatting and parsing. + ([Pull Request](https://github.com/rails/rails/pull/16917)) + Credits ------- diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index b89485b945..f914122242 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -247,6 +247,7 @@ end ``` This migration will create a `user_id` column and appropriate index. +For more `add_reference` options, visit the [API documentation](http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-add_reference). There is also a generator which will produce join tables if `JoinTable` is part of the name: @@ -355,6 +356,13 @@ end will append `ENGINE=BLACKHOLE` to the SQL statement used to create the table (when using MySQL or MariaDB, the default is `ENGINE=InnoDB`). +Also you can pass the `:comment` option with any description for the table +that will be stored in database itself and can be viewed with database administration +tools, such as MySQL Workbench or PgAdmin III. It's highly recommended to specify +comments in migrations for applications with large databases as it helps people +to understand data model and generate documentation. +Currently only the MySQL and PostgreSQL adapters support comments. + ### Creating a Join Table The migration method `create_join_table` creates an HABTM (has and belongs to @@ -454,6 +462,7 @@ number of digits after the decimal point. are using a dynamic value (such as a date), the default will only be calculated the first time (i.e. on the date the migration is applied). * `index` Adds an index for the column. +* `comment` Adds a comment for the column. Some adapters may support additional options; see the adapter specific API docs for further information. diff --git a/guides/source/api_app.md b/guides/source/api_app.md index 8dba914923..e50a24ce55 100644 --- a/guides/source/api_app.md +++ b/guides/source/api_app.md @@ -79,8 +79,6 @@ Handled at the middleware layer: code. All you need to do is use the [`stale?`](http://api.rubyonrails.org/classes/ActionController/ConditionalGet.html#method-i-stale-3F) check in your controller, and Rails will handle all of the HTTP details for you. -- Caching: If you use `dirty?` with public cache control, Rails will automatically - cache your responses. You can easily configure the cache store. - HEAD requests: Rails will transparently convert `HEAD` requests into `GET` ones, and return just the headers on the way out. This makes `HEAD` work reliably in all Rails APIs. diff --git a/railties/lib/rails/railtie.rb b/railties/lib/rails/railtie.rb index d03f8324bc..492c519222 100644 --- a/railties/lib/rails/railtie.rb +++ b/railties/lib/rails/railtie.rb @@ -1,5 +1,4 @@ require 'rails/initializable' -require 'rails/configuration' require 'active_support/inflector' require 'active_support/core_ext/module/introspection' require 'active_support/core_ext/module/delegation' @@ -112,7 +111,7 @@ module Rails # # Be sure to look at the documentation of those specific classes for more information. class Railtie - autoload :Configuration, "rails/railtie/configuration" + autoload :Configuration, 'rails/railtie/configuration' include Initializable diff --git a/railties/test/application/bin_setup_test.rb b/railties/test/application/bin_setup_test.rb index a07c51a60f..ba700df1d6 100644 --- a/railties/test/application/bin_setup_test.rb +++ b/railties/test/application/bin_setup_test.rb @@ -43,6 +43,8 @@ module ApplicationTests The Gemfile's dependencies are satisfied == Preparing database == +Created database 'db/development.sqlite3' +Created database 'db/test.sqlite3' == Removing old logs and tempfiles == diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb index a229609e84..cee9db5535 100644 --- a/railties/test/application/rake/dbs_test.rb +++ b/railties/test/application/rake/dbs_test.rb @@ -29,11 +29,11 @@ module ApplicationTests def db_create_and_drop(expected_database) Dir.chdir(app_path) do output = `bin/rails db:create` - assert_empty output + assert_match(/Created database/, output) assert File.exist?(expected_database) assert_equal expected_database, ActiveRecord::Base.connection_config[:database] output = `bin/rails db:drop` - assert_empty output + assert_match(/Dropped database/, output) assert !File.exist?(expected_database) end end diff --git a/railties/test/generators/channel_generator_test.rb b/railties/test/generators/channel_generator_test.rb index 23d0c7b4a4..d58b54ac24 100644 --- a/railties/test/generators/channel_generator_test.rb +++ b/railties/test/generators/channel_generator_test.rb @@ -46,4 +46,16 @@ class ChannelGeneratorTest < Rails::Generators::TestCase assert_file "app/assets/javascripts/cable.js" end + + def test_channel_on_revoke + run_generator ['chat'] + run_generator ['chat'], behavior: :revoke + + assert_no_file "app/channels/chat_channel.rb" + assert_no_file "app/assets/javascripts/channels/chat.coffee" + + assert_file "app/channels/application_cable/channel.rb" + assert_file "app/channels/application_cable/connection.rb" + assert_file "app/assets/javascripts/cable.js" + end end |