activerecord-typedstore icon indicating copy to clipboard operation
activerecord-typedstore copied to clipboard

typed_store attributes is marked as changed when inspecting instance

Open adis-io opened this issue 7 years ago • 10 comments

Steps to reproduce

  1. save snippet below, name it like json_typed_store_bug.rb
  2. ruby json_typed_store_bug.rb
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', '4.2.7.1'
  gem 'pg', '0.19.0'
  gem 'factory_bot_rails'
  gem 'activerecord-typedstore'
end

require 'active_record'
require 'action_controller/railtie'

ActiveRecord::Base.establish_connection(
  adapter: 'postgresql', database: 'ovkovckm',
  host: 'baasu.db.elephantsql.com', username: 'ovkovckm',
  password: '866lKeR6FSUgG_fEfo1e13XpYu0zZ-Iz'
)
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :users, force: true do |t|
    t.string :username
    t.json :extra_attrs, default: {}, null: false
    t.timestamps
  end
end

class User < ActiveRecord::Base
  typed_store :extra_attrs do |s|
    s.string :first_name
    s.string :last_name
  end
end

FactoryBot.define do
  factory :user do
    username { 'username' }
  end
end

require 'minitest/autorun'

class TypedStoreTest < Minitest::Test
  def test_primary_categories
    FactoryBot.create(:user)

    user = User.first
    assert_equal({}, user.changed_attributes)

    user.inspect

    assert_equal({}, user.changed_attributes)
  end
end

Expected behaviour

tests passes

Actual behaviour

tests fails

adis-io avatar Nov 30 '17 04:11 adis-io

I didn't know how to reproduce without user interaction. I will appreciate if somebody will give a hint.

adis-io avatar Nov 30 '17 04:11 adis-io

Try changing biding.pry with user.inspect

rafaelfranca avatar Dec 01 '17 16:12 rafaelfranca

Thank you, @rafaelfranca

So when I type user it inspects object?

adis-io avatar Dec 02 '17 21:12 adis-io

Yes

rafaelfranca avatar Dec 05 '17 14:12 rafaelfranca

describe 'dirty tracking' do
  it 'is not dirty when freshly initialized' do
    model = described_class.new
    model.inspect
    expect(model.changed?).to be_falsey
  end
end

Test case not passing by now for models which use typed_store.

I think model should not be marked as dirty when you inspect it.

What do you think, guys? @rafaelfranca @byroot

adis-io avatar Jul 20 '18 11:07 adis-io

Just ran into a version of this where a freshly loaded model is reporting as changed.

class Dummy < ActiveRecord::Base
  typed_store :fields, coder: ::ActiveRecord::TypedStore::IdentityCoder do |s|
    s.string :email
  end
end

d = Dummy.last
d.changed? # => true

before = d.attribute_in_database(:fields)
after = d.fields
before == after # => true

gaffneyc avatar Jun 14 '19 18:06 gaffneyc

@gaffneyc am also running into a similar issue, though in my case, the ActiveRecord instance is only marked as dirty once I access any typed_store field.

I believe the problem stems from how ActiveModel::Attribute works:

d.send(:mutations_from_database).send(:attributes)["email"].send(:original_value_for_database) will return the serialized version of the field, which will always be different from the “apparent”, deserialized version of that field, which ActiveModel recognizes as changed_in_place? == true

I’ll investigate if we can sidestep this somehow.

edward avatar Sep 10 '19 20:09 edward

This behavior is conflicting with activerecord's locking. To extend @gaffneyc's example, if you try to run this:

d = Dummy.last
d.with_lock do
  # Doesn't matter what's in here
end

then ActiveRecord will throw an error:

RuntimeError: Locking a record with unpersisted changes is not supported. Use `save` to persist the changes, or `reload` to discard them explicitly.

gilbert avatar May 18 '20 03:05 gilbert

+1

mountriv99 avatar Aug 07 '20 16:08 mountriv99

+1

bartekadams avatar Aug 19 '20 11:08 bartekadams