Dup'ing models doesn't copy attributes

[husam212]

class TestModel
  include AttrJson::Model
  attr_json :test, :string
end

foo = TestModel.new(test: 'foo')
bar = foo.dup
bar.test = 'bar'
foo.test #=> bar

Same behavior with clone and deep_dup.

[jrochkind]

Hmm. So the dup'd model shares a reference to some same object internally.

Would you mind checking to see if this behavior is the same without attr_json, but with a straight :json or :jsonb column? attr_json is intending not to keep state anywhere but that actual original json/jsonb hash in the model. So I wonder if the problem is there with straight ActiveRecord and a json/jsonb column too.

If it is, there isn't much attr_json can/will do about it, and I'd try filing it as a 'bug' with AR.

If the problem isn't there like that though, there might be something we can do about it.

[husam212]

I checked the behavior of json and jsonb columns without using attr_json, both behaves as expected, so I think this issue is caused by attr_json.

[jrochkind]

Thanks for checking. That's too bad.

I'll try to find time to diagnose and see if I can find a fix. If anyone else wants to do more investigation toward that too, it would be appreciated!

[jrochkind]

To investigate and make sure I understand the baseline of how standard ActiveRecord handles standard json or jsonb-typed hashes, I created a model in a blank Rails app with:

rails g model product title:string data:jsonb

Executed my migrations. Then, here's what I see:

irb(main):003:0> product = Product.create(title: "my product", data: { "key" => "value" })
   (0.2ms)  BEGIN
  Product Create (1.2ms)  INSERT INTO "products" ("title", "data", "created_at", "updated_at") VALUES ($1, $2, $3, $4) RETURNING "id"  [["title", "my product"], ["data", "{\"key\":\"value\"}"], ["created_at", "2020-09-01 21:30:18.455302"], ["updated_at", "2020-09-01 21:30:18.455302"]]
   (1.3ms)  COMMIT
=> #<Product id: 1, title: "my product", data: {"key"=>"value"}, created_at: "2020-09-01 21:30:18", updated_at: "2020-09-01 21:30:18">
irb(main):004:0> cloned_product = product.clone
=> #<Product id: 1, title: "my product", data: {"key"=>"value"}, created_at: "2020-09-01 21:30:18", updated_at: "2020-09-01 21:30:18">
irb(main):005:0> cloned_product.data["new key"] = "new value"
=> "new value"
irb(main):006:0> product.data
=> {"key"=>"value", "new key"=>"new value"}

You see that the hash stored in data ends up shared between the two objects, after a clone.

However, that does not seem to be the same with dup with a straight jsonb hash.

irb(main):007:0> duped_product = product.dup
=> #<Product id: nil, title: "my product", data: {"key"=>"value", "new key"=>"new value"}, created_at: nil, updated_at: nil>
irb(main):008:0> duped_product.data["duped new key"] = "duped new value"
=> "duped new value"
irb(main):009:0> product.data
=> {"key"=>"value", "new key"=>"new value"}
irb(main):010:0> duped_product.data
=> {"key"=>"value", "new key"=>"new value", "duped new key"=>"duped new value"}

So I guess that is different if attr_json actually behaves differnetly with dup -- but the unexpected behavior is only with dup, with clone it's expected.

I am going to change the title of this issue to "dup'ing models doesn't copy attributes", because I don't think we expect cloning too, based on standard Rails behavior.

OK, I still need to investigate more with behavior of attr_json under dup then.

[jrochkind]

Oh, I just noticed that you are specifically talking about an AttrJson::Model, not an actual ActiveRecord. I had misunderstood that.

I am going to edit the title to "Dup'ing AttrJson::Models doesn't copy attributes", to be clear that we're talking about dup'ing -- I don't think cloning should be expected to -- and that we're talking about AttrJson::Model, not ActiveRecord models using AttrJson::Record.

I am not totally sure if this is a bug or feature request, I am not sure what we should expect dup or clone to do here.

Can you say more about your use case, what you are doing and why you expect this?

Here is an easy workaround to let you make the kind of no-shared-state dup you want:

foo = TestModel.new(test: 'foo')
bar = TestModel.new(foo.as_json)
bar.test = 'bar'
foo.test #=> 'foo'

I suppose I could implement dup to do that... I'm not totally sure if I should?

You could always do it yourself if you want, something like:

class TestModel
  include AttrJson::Model
  attr_json :test, :string

  def dup
     self.class.new(self.as_json)
  end
end

[jrochkind]

OK, did more research and thought. Dup and clone are confusing in ruby.

Because AttrJson::Model stores everything in an underlying single hash, and that single hash isn't really "deep" dup'd, it ends up sharing state, when it doesn't seem like it "should", because "simple" attributes, while they are shared, will wind up "un-shared" when you set a new thing with an assignment. This is confusing to talk about.

But okay, this might be the right thing to do to fix, attr_json should be changed so AttrJson::Model has a method like this

  def initialize_copy(other)
    @attributes = other.attributes.deep_dup
    super
  end

(Or maybe that should be initialize_dup to fix dup and not clone like Rails does, not sure why Rails chose that).

Have to think about it a bit more, but that would probably take care of it. Not sure if it's dangerous in some way, perhaps performance-related, though. Want to look more at what Rails does in various places.

Still curious to hear about your use case and how you ran into this.

Longer-term, AttrJson::Model should probably converge more with ActiveModel::Attributes -- which didn't exist when I first wrote attr_json.

[husam212]

My use case is some kind of inheritance where the inherited model apply some modifiers to its parent, so the logic dup (or clone) the parent and then apply the modifiers, in this use case the parent also changes while it shouldn't.

I'm currently patching it using your suggestion in https://github.com/jrochkind/attr_json/issues/96#issuecomment-685150614.

[jrochkind]

It's been a while, but I have #dup in #169

That also makes #deep_dup happen properly, as it's by default just a synonym for #dup. Both will do a deep dup now.

clone right now does NOT copy attributes, both instances end up with references to the same attributes. But that seems to be consistent with what ActiveModel::Model/ActiveModel::Attributes does. So that seems like it should remain as it is.