Compatibility with ActiveModel::Dirty

[slowjack2k]

When I try to use ActiveModel::Dirty with virtus I get a Stack Level to deep error.

This happens because ActiveModel::Dirty overrides the accessors for properties to use the Attributes-Hash (some how, some where) and this virtus code:

# attribute_set.rb

   def get(object)
      each_with_object({}) do |attribute, attributes|
        name = attribute.name
        attributes[name] = object.__send__(name) if attribute.public_reader?
      end
    end

results to in an invinite loop.

Is it possible to change this method like this?

    def get(object)
      each_with_object({}) do |attribute, attributes|
        name = attribute.name
        attributes[name] = attribute.get(object) if attribute.public_reader?
      end
    end

Also ActiveModel::Dirty need's in conjunction with viruts an method attribute (I couldn't figure out why) like this:

  def attribute(name, *args)
      attribute_set[name.to_sym].get(self)
  end

Can virtus provide this method?

Regards Dieter

[solnic]

Can you make a script that shows the error?

We can add some integration code required by AS but I'd prefer to have that in a separate gem. virtus-active_support or something like that.

[slowjack2k]

This is what I would expect

require 'spec_helper'
require 'active_model'
require 'virtus'

describe "ActiveModel::Dirty support" do

class User
  include ::Virtus.model

  include ActiveModel::Model
  include ActiveModel::Dirty

  def self.attribute(attr_name, *new_properties)
    super

    define_attribute_methods attr_name

    method_str = <<-"EOF_M"
         def #{attr_name}=(new_value)
           #{attr_name}_will_change! unless new_value == attribute_set[:#{attr_name}].get(self)
           super
         end
    EOF_M

    class_eval method_str, __FILE__, __LINE__ + 1

  end

  attribute :id, Integer
  attribute :name, String

end

  subject{
    User.new
  }

  it "tracks model changes" do

    subject.id = 1
    expect(subject.changes).to include({:id => [nil, 1]})
  end

end

But I still ask my self what attributes should return. With the current implementation it returns what ever the current getter returns. So when I override the getter like this:

  def my_attr
     attribute_set[:my_attr].get(self).to_s + "some thing else"
  end

attributes returns my decorated value.

[solnic]

#attributes returns whatever readers return, not ivar values. It feels like it requires some level of integration so as I said - this could be done in a separate project.

[slowjack2k]

Ok, then there is a clash between what Rails see's as attributes and what virtus see's as attributes.

In this case I think the separate gem should create all method's which define_attribute_methods creates (ActiveModel::AttributeMethods) in a virtus compatible way. define_attribute_methodsis the macro which creates the incompatible method's. ActiveModel::Dirty uses this method's later.

[solnic]

Actually I'm gonna close this one. I think the stack error is caused by a conflict between AS modules and Virtus.

FWIW this way of tracking dirty state sucks big time that's why we're not doing it like that in ROM and that's why it is not supported by virtus. It's not stable and the #attribute_will_change! feels more like a workaround than a solution.

Anyway, if you find a way to fix stack error and it turns out that's a bug in virtus (I doubt that but who knows) I'll be happy to accept a patch.

[mbj]

@solnic Thx. ARs dirty tracking is some public API that makes internals even more complex. Thx for rejecting this.

[slowjack2k]

JM2C, I think the goal of rom is awesome and you can think of AR's dirty tracking & AR what ever you want.

But I think rom's succes will depent on the compatibility with Rails. When you say Ruby most people think of Rails and not chef or sinatra or some thing else.

So in the end you have to care about compatibility with Rails, Rails wan't care about compatibility with rom.

[solnic]

I'm not sure how mixing dirty-tracking into Virtus will help in rails integration. FWIW I think we will need a set of different tools to integrate with Rails.

For instance for AM compatibility we will have to introduce some rom-rails library that adds various mixins and new objects which can work with Rails helpers etc.

I don't think polluting virtus itself with extra functionality is needed here.

So yes, I agree we need rom to work with rails, no doubt about that but it comes to specifics how we want to do that.

[mbj]

@slowjack2k We can easily build an AR like object. That uses virtus an reference to the ROM session etc. But I agree with @solnic that this should NOT be part of virtus.

[benjaminbarbe]

@slowjack2k How did you solve your problem?

[slowjack2k]

@benjaminbarbe Here you can find a gist

[mbj]

I think the fundamental concept of 'dirty tracking' is fundamentally borked. Better to use 'dirty detection' ad adapter / session / UoW layer. Where the original DB side dump is available for comparison.

[benjaminbarbe]

@slowjack2k Thanks a lot! @mbj Yes it's true. But now gems continue to use it. So for compatibility is required :confused:

[solnic]

If it's a bug in virtus then I'm more than happy to accept a patch :)

On Sat, Jun 28, 2014 at 4:51 PM, benjaminbarbe notifications@github.com wrote:

@slowjack2k Thanks a lot!

@mbj Yes it's true. But now gems continue to use it. So for compatibility is required :confused:

Reply to this email directly or view it on GitHub: https://github.com/solnic/virtus/issues/239#issuecomment-47422006

[slowjack2k]

@solnic we did discuss this in the beginning of this issue. Nothing changed. So there is no need to discuss it again.