diff options
80 files changed, 1370 insertions, 266 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 595830feb0..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 @@ -412,12 +412,12 @@ The above will start a cable server on port 28080. ### In app -If you are using a server that supports the [Rack socket hijacking API](http://www.rubydoc.info/github/rack/rack/file/SPEC#Hijacking), Action Cable can run alongside your Rails application. For example, to listen for WebSocket requests on `/cable`, mount the server at that path: +If you are using a server that supports the [Rack socket hijacking API](http://www.rubydoc.info/github/rack/rack/file/SPEC#Hijacking), Action Cable can run alongside your Rails application. For example, to listen for WebSocket requests on `/websocket`, specify that path to `config.action_cable.mount_path`: ```ruby -# config/routes.rb -Example::Application.routes.draw do - mount ActionCable.server => '/cable' +# config/application.rb +class Application < Rails::Application + config.action_cable.mount_path = '/websocket' end ``` 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 f654ce0bfa..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,15 +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 handler = callback || block - handler = -> message { handler.(coder.decode(message)) } if coder - 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 @@ -117,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 f51f19eb7d..0b0c72ccf6 100644 --- a/actioncable/test/channel/stream_test.rb +++ b/actioncable/test/channel/stream_test.rb @@ -116,11 +116,22 @@ module ActionCable::StreamTests require 'action_cable/subscription_adapter/inline' + class UserCallbackChannel < ActionCable::Channel::Base + def subscribed + stream_from :channel do + Thread.current[:ran_callback] = true + end + end + end + class StreamEncodingTest < ActionCable::TestCase setup do @server = TestServer.new(subscription_adapter: ActionCable::SubscriptionAdapter::Inline) @server.config.allowed_request_origins = %w( http://rubyonrails.com ) - @server.stubs(:channel_classes).returns(ChatChannel.name => ChatChannel) + @server.stubs(:channel_classes).returns( + ChatChannel.name => ChatChannel, + UserCallbackChannel.name => UserCallbackChannel, + ) end test 'custom encoder' do @@ -131,6 +142,18 @@ 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 + + test "user supplied callbacks are run through the worker pool" do + run_in_eventmachine do + connection = open_connection + receive(connection, command: 'subscribe', channel: UserCallbackChannel.name, identifiers: { id: 1 }) + + @server.broadcast 'channel', {} + wait_for_async + refute Thread.current[:ran_callback], "User callback was not run through the worker pool" end end @@ -151,8 +174,8 @@ module ActionCable::StreamTests end end - def receive(connection, command:, identifiers:) - identifier = JSON.generate(channel: 'ActionCable::StreamTests::ChatChannel', **identifiers) + def receive(connection, command:, identifiers:, channel: 'ActionCable::StreamTests::ChatChannel') + identifier = JSON.generate(channel: channel, **identifiers) connection.dispatch_websocket_message JSON.generate(command: command, identifier: identifier) wait_for_async 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/connection/client_socket_test.rb b/actioncable/test/connection/client_socket_test.rb index dd730e348f..4af071b4da 100644 --- a/actioncable/test/connection/client_socket_test.rb +++ b/actioncable/test/connection/client_socket_test.rb @@ -1,10 +1,9 @@ require 'test_helper' require 'stubs/test_server' -class ActionCable::Connection::StreamTest < ActionCable::TestCase +class ActionCable::Connection::ClientSocketTest < ActionCable::TestCase class Connection < ActionCable::Connection::Base - attr_reader :websocket, :subscriptions, :message_buffer, :connected - attr_reader :errors + attr_reader :connected, :websocket, :errors def initialize(*) super diff --git a/actioncable/test/connection/stream_test.rb b/actioncable/test/connection/stream_test.rb index d5aad63648..a7a61d8d6f 100644 --- a/actioncable/test/connection/stream_test.rb +++ b/actioncable/test/connection/stream_test.rb @@ -3,8 +3,7 @@ require 'stubs/test_server' class ActionCable::Connection::StreamTest < ActionCable::TestCase class Connection < ActionCable::Connection::Base - attr_reader :websocket, :subscriptions, :message_buffer, :connected - attr_reader :errors + attr_reader :connected, :websocket, :errors def initialize(*) super 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/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 64672de57e..f9b80dd805 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -756,6 +756,10 @@ module ActionController end end + def non_scalar?(value) + value.is_a?(Array) || value.is_a?(Parameters) + end + EMPTY_ARRAY = [] def hash_filter(params, filter) filter = filter.with_indifferent_access @@ -770,7 +774,7 @@ module ActionController array_of_permitted_scalars?(self[key]) do |val| params[key] = val end - else + elsif non_scalar?(value) # Declaration { user: :name } or { user: [:name, :age, { address: ... }] }. params[key] = each_element(value) do |element| element.permit(*Array.wrap(filter[key])) 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/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 96048e2868..b75eb0e3bf 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -360,4 +360,13 @@ class ParametersPermitTest < ActiveSupport::TestCase assert @params.include? 'person' assert_not @params.include? :gorilla end + + test "scalar values should be filtered when array or hash is specified" do + params = ActionController::Parameters.new(foo: "bar") + + assert params.permit(:foo).has_key?(:foo) + refute params.permit(foo: []).has_key?(:foo) + refute params.permit(foo: [:bar]).has_key?(:foo) + refute params.permit(foo: :bar).has_key?(:foo) + end end 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 0e831bfb66..16ce131cf4 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,82 @@ +* 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 + date part of the string when it coerces to time. + + *Ryuta Kamizono* + * Properly accept all valid JSON primitives in the JSON data type. Fixes #24234 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/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..ce2f9f1925 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -5,7 +5,7 @@ module ActiveRecord 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 +15,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 +23,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/quoting.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb index d3a91f73c8..d5a181d3e2 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb @@ -14,6 +14,10 @@ module ActiveRecord @quoted_column_names[name] ||= %Q("#{super.gsub('"', '""')}") end + def quoted_time(value) + quoted_date(value) + end + private def _quote(value) 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/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/adapters/sqlite3/quoting_test.rb b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb index 87a892db37..f3ec2b98d3 100644 --- a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb @@ -8,9 +8,7 @@ module ActiveRecord class SQLite3Adapter class QuotingTest < ActiveRecord::SQLite3TestCase def setup - @conn = Base.sqlite3_connection :database => ':memory:', - :adapter => 'sqlite3', - :timeout => 100 + @conn = ActiveRecord::Base.connection end def test_type_cast_binary_encoding_without_logger @@ -89,6 +87,13 @@ module ActiveRecord assert_equal "'hello'", @conn.quote(type.serialize(value)) end + + def test_quoted_time_returns_date_qualified_time + value = ::Time.utc(2000, 1, 1, 12, 30, 0, 999999) + type = Type::Time.new + + assert_equal "'2000-01-01 12:30:00.999999'", @conn.quote(type.serialize(value)) + end end end end 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/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/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 5729a98b99..58ec6fb4fe 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 causes 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/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index 8a74ad4d66..f297214d0f 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 true) + class Array + def sum(*args) #:nodoc: + # Use Enumerable#sum instead. + super + end + end +end diff --git a/activesupport/lib/active_support/core_ext/hash/keys.rb b/activesupport/lib/active_support/core_ext/hash/keys.rb index 8b2366c4b3..1bfa18aeee 100644 --- a/activesupport/lib/active_support/core_ext/hash/keys.rb +++ b/activesupport/lib/active_support/core_ext/hash/keys.rb @@ -11,7 +11,7 @@ class Hash # hash.transform_keys.with_index { |k, i| [k, i].join } # => {"name0"=>"Rob", "age1"=>"28"} def transform_keys return enum_for(:transform_keys) { size } unless block_given? - result = self.class.new + result = {} each_key do |key| result[yield(key)] = self[key] end 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/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/core_ext/hash/transform_keys_test.rb b/activesupport/test/core_ext/hash/transform_keys_test.rb index 99af274614..962d3a30b6 100644 --- a/activesupport/test/core_ext/hash/transform_keys_test.rb +++ b/activesupport/test/core_ext/hash/transform_keys_test.rb @@ -43,4 +43,20 @@ class TransformKeysTest < ActiveSupport::TestCase original.transform_keys!.with_index { |k, i| [k, i].join.to_sym } assert_equal({ a0: 'a', b1: 'b' }, original) end + + test "transform_keys returns a Hash instance when self is inherited from Hash" do + class HashDescendant < ::Hash + def initialize(elements = nil) + super(elements) + (elements || {}).each_pair{ |key, value| self[key] = value } + end + end + + original = HashDescendant.new({ a: 'a', b: 'b' }) + mapped = original.transform_keys { |k| "#{k}!".to_sym } + + assert_equal({ a: 'a', b: 'b' }, original) + assert_equal({ a!: 'a', b!: 'b' }, mapped) + assert_equal(::Hash, mapped.class) + end end 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/4_0_release_notes.md b/guides/source/4_0_release_notes.md index b9444510ea..4615cf18e6 100644 --- a/guides/source/4_0_release_notes.md +++ b/guides/source/4_0_release_notes.md @@ -108,7 +108,7 @@ In Rails 4.0, several features have been extracted into gems. You can simply add * Mass assignment protection in Active Record models ([GitHub](https://github.com/rails/protected_attributes), [Pull Request](https://github.com/rails/rails/pull/7251)) * ActiveRecord::SessionStore ([GitHub](https://github.com/rails/activerecord-session_store), [Pull Request](https://github.com/rails/rails/pull/7436)) * Active Record Observers ([GitHub](https://github.com/rails/rails-observers), [Commit](https://github.com/rails/rails/commit/39e85b3b90c58449164673909a6f1893cba290b2)) -* Active Resource ([GitHub](https://github.com/rails/activeresource), [Pull Request](https://github.com/rails/rails/pull/572), [Blog](http://yetimedia.tumblr.com/post/35233051627/activeresource-is-dead-long-live-activeresource)) +* Active Resource ([GitHub](https://github.com/rails/activeresource), [Pull Request](https://github.com/rails/rails/pull/572), [Blog](http://yetimedia-blog-blog.tumblr.com/post/35233051627/activeresource-is-dead-long-live-activeresource)) * Action Caching ([GitHub](https://github.com/rails/actionpack-action_caching), [Pull Request](https://github.com/rails/rails/pull/7833)) * Page Caching ([GitHub](https://github.com/rails/actionpack-page_caching), [Pull Request](https://github.com/rails/rails/pull/7833)) * Sprockets ([GitHub](https://github.com/rails/sprockets-rails)) 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/action_cable_overview.md b/guides/source/action_cable_overview.md index d1f17fdce5..0c486bb96c 100644 --- a/guides/source/action_cable_overview.md +++ b/guides/source/action_cable_overview.md @@ -549,12 +549,13 @@ You can change that in `config/database.yml` through the `pool` attribute. ### In App Action Cable can run alongside your Rails application. For example, to -listen for WebSocket requests on `/websocket`, mount the server at that path: +listen for WebSocket requests on `/websocket`, specify that path to +`config.action_cable.mount_path`: ```ruby -# config/routes.rb -Example::Application.routes.draw do - mount ActionCable.server => '/cable' +# config/application.rb +class Application < Rails::Application + config.action_cable.mount_path = '/websocket' end ``` 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/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index d14c9158ca..34cfb742a4 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -44,7 +44,7 @@ TIP: Ruby 1.8.7 p248 and p249 have marshaling bugs that crash Rails. Ruby Enterp ### The Task -Rails provides the `app:update` task. After updating the Rails version +Rails provides the `app:update` task (`rails:update` on 4.2 and earlier). After updating the Rails version in the Gemfile, run this task. This will help you with the creation of new files and changes of old files in an interactive session. 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 |