New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`touch` leaves model with optimistic locking in changed state #26496

Closed
koppen opened this Issue Sep 14, 2016 · 1 comment

Comments

Projects
None yet
3 participants
@koppen
Contributor

koppen commented Sep 14, 2016

TLDR: Using touch in Rails 5.0.0.1 leaves the ActiveRecord models with lock_version in a changed state:

Steps to reproduce

Set up a model with optimistic locking in a blank Rails application:

$ rails generate model Hammer lock_version
$ rails db:migrate
$ rails console

Create a new record and touch it:

2.3.0 :001 > mc = Hammer.create!
 => #<Hammer id: 1, lock_version: 0, created_at: "2016-09-14 14:06:12", updated_at: "2016-09-14 14:06:12">
2.3.0 :002 > mc.lock_version
 => 0
2.3.0 :003 > mc.touch
 => true

Actual behaviour

2.3.0 :004 > mc.changed?
 => true
2.3.0 :005 > mc.changes
 => {"lock_version"=>[0, "1"]}

Expected behaviour

2.3.0 :004 > mc.changed?
 => false
2.3.0 :005 > mc.changes
 => {}

A complete reproduction script can be found below.

System configuration

$ rails -v
Rails 5.0.0.1

$ ruby -v
ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin15]

Reproduction script

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"
  gem "rails", github: "rails/rails"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :hammers, force: true do |t|
    t.integer :lock_version
    t.timestamps
  end
end

class Hammer < ActiveRecord::Base
end

class TouchTest < Minitest::Test
  def test_touching_stuff
    hammer = Hammer.create!

    hammer.touch

    assert_equal 1, hammer.lock_version
    refute hammer.changed?, "Changes should be cleared: #{hammer.changes.inspect}"
  end
end
@alexcameron89

This comment has been minimized.

Member

alexcameron89 commented Sep 14, 2016

The executable script passes on 4.2.7 and fails on master. It looks like issue occurred from 5e8d96c.

@koppen's PR fixes the behavior. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment