Incomplete index in Callback

[samqiu]

Hi @karmi ,

Thank you for your awesome gems!

I am replacing Tire to elasticsearch-rails, the first issue is that it won't perform as_indexed_json when update_document, It' s update changed_attributes only.

Is there any solution about this? perform as_indexed_json when update_document, not only changed_attributes .

Source

https://github.com/elasticsearch/elasticsearch-rails/blob/master/elasticsearch-model/lib/elasticsearch/model/indexing.rb#L334

Model

class User << ActiveRecord::Base

  has_many :keywords

  def as_indexed_json(options={})
    { :id => id, 
      email: email,
      first_name: first_name, 
      last_name: last_name,

      full_name: "#{first_name} #{last_name}",
      research_interests: keywords.map(&:name)
    }
  end

end

[karmi]

Hi, thanks! Can you clarify the issue a bit, please?

The thing is, that -- contrary to Tire --, update_document intercepts model changes, via `ActiveModel::Dirty``, and performs a partial update of only changed properties.

So, are you saying that some of these changes are not correctly intercepted? Would it be feasible to model it in the integration test?

[samqiu]

Thanks!

elasticsearch-rails is correctly updated the changed attributes.

But some field also need to update. like the full_name below.

class User << ActiveRecord::Base

  def as_indexed_json(options={})
    { :id => id,  
      first_name: first_name, 
      last_name: last_name,
      full_name: "#{first_name} #{last_name}", 
    }
  end

end

So, to avoid more problems, I think is better invoke as_indexed_json when perform update_document, not just update the changed_attributes.

Hope you can understand my poor English.

[karmi]

... full_name: "#{first_name} #{last_name}", ...

Aaah, now I know what you mean -- full_name is defined only in as_indexed json, so it is not intercepted in changed attributes...

I think is better invoke as_indexed_json when perform update_document, not just updated the changed_attributes

Yeah, but that would defeat the purpose of the partial update I guess :) I'll have to revisit this part...

[samqiu]

Exactly!

But I think the partial update should be based on as_indexed_json.

[karmi]

But I think the partial update should be based on as_indexed_json

Unless I'm mistaken, that wouldn't help, since the full_author method wouldn't appear in the changed attributes. And if we would simply call as_indexed_json, then we're sending the full model serialization over the wire, defeating the purpose of the partial update?

[samqiu]

Thanks,

Temporary using this to fix it :

after_update -> {
  __elasticsearch__.index_document
}

[samqiu]

Should I reopen this issue for tracking?

[karmi]

I still haven't found a way how to solve this acceptably, please see my comments above.

Unless there some kind of support for "computed" or "dependent" model attributes (a la e.g. Ember.js), these derived attributes simply are not reachable by ActiveModel::Dirty.

I can see an argument that as soon as you define as_indexed_json on your model, the callback should automatically call index_document. On the other hand, I wouldn't like making the automatic callbacks so magical and smart, they handle every use case... I've seen much of these issues and participated in many discussions like this in Tire, and unless there's a clear way, it's usually best leaving this to the user?

[karmi]

So, for the record, the trouble with your use case is that full_name is constructed on the fly, during as_indexed_json call. In this case, you simply have to take indexing in your own hands.

If it were a "read-only", or "computed" attribute, such as:

class Person < ActiveRecord::Base
  def full_name
    [first_name, last_name].compact.join(' ')
  end
end

which would be used in as_indexed_json, then you'd have to redefine first_name and last_name accessors to call attribute_will_change!. There's a nice StackOverflow discussion: Track dirty for not-persisted attribute in an ActiveRecord object in Rails about a similar use case.

So, in my opinion, the library behaves absolutely predictably here.

[samqiu]

@karmi

Now I know what to do, thank you!

[karmi]

@samqiu Please see the linked commit, it should solve one variant of the bug you have hit. I'm afraid, though, that your specific case, ie. having a custom method, which is then used in the as_indexed_json, simply requires defining the callbacks on your own.

Let me repeat once more that the "automatic callbacks" were always meant as a convenience -- for most complex cases, you need to define them, preferably via Sidekiq/etc...

[samqiu]

Thank you. I will going to use Sidekiq for indexing.