Skip to content
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

as_json only: [...attributs...], methods: [...methods list...] => callback update_document does not update "methods values" in ES #40

Closed
alotela opened this issue Mar 12, 2014 · 9 comments

Comments

@alotela
Copy link

alotela commented Mar 12, 2014

Hi,

I have a mapping with methods and attributes:
def as_indexed_json options={}
as_json only: ['id', 'banned','birthday'],
methods: ['method1', 'method2', methodn']
end

when I want to update a document, only changed attributes are saved in ES, not methods...
Looking at indexing.rb code, this seems normal:
def update_document(options={})
if changed_attributes = self.instance_variable_get(:@__changed_attributes)
client.update(
{ index: index_name,
type: document_type,
id: self.id,
body: { doc: changed_attributes } }.merge(options)
)
else
index_document(options)
end
end

so, how can I autosave "methods" values to ES when they are changedat the same time as attributes ?

@karmi
Copy link
Contributor

karmi commented Mar 12, 2014

Implement you own callback for save or commit lifecycle events.

@alotela
Copy link
Author

alotela commented Mar 12, 2014

ok. It would be nice if we could say 'if this attribute changes, then save this method at the same time'

@karmi
Copy link
Contributor

karmi commented Mar 12, 2014

There are couple of issues opened for that, lately: #5 and #37. We need to better document it, or improve the behaviour.

@alotela
Copy link
Author

alotela commented Mar 12, 2014

ok. thank you. We'l try to help with a solution if I find something interesting.

@alotela alotela closed this as completed Mar 12, 2014
karmi added a commit that referenced this issue Apr 15, 2014
…serialized during update

Previously, the `update_document` method simply intercepted the changes to the model, via the
`@__changed_attributes` variable, and used these directly.

This caused models with a custom serialization method to be incorrectly serialized, namely
unwanted attributes were added.

This patch looks for `as_indexed_json` defined on the model, and when it finds it,
filters the changed attributes through the keys.

Closes #75

Related:

* #59
* #57
* #52
* #40
* #37
* #5
@karmi
Copy link
Contributor

karmi commented Apr 15, 2014

@alotela The linked commit should fix your issue.

@alotela
Copy link
Author

alotela commented Apr 16, 2014

If I'm right, it just checks for attributes I really want to put into ES. My problem is that when I use methods array in as_indexed_json, they are not updated. It's not about unwanted attributes.
I solved the problem with this:
1/ I Changed the method:

    module Elasticsearch
      module Model
        module Indexing
          module InstanceMethods
            def update_document(options={})
              changed_attributes = self.instance_variable_get(:@__changed_attributes)
              changed_attributes.merge!(es_changed_methods_values(changed_attributes.keys)) if changed_attributes
              if changed_attributes
                client.update(
                  { 
                     index: index_name, type:  document_type, id:    self.id, body:  { doc: changed_attributes }}.merge(options)
                )
                else
                  index_document(options)
                end
              end

               def es_changed_methods_values changed_attributs
                  changed_methods = {}
                  return changed_methods unless respond_to?(:es_changed_methods)
              es_changed_methods.each do |attribut, methods|
                    methods.each { |method| changed_methods[method] = send(method) } if changed_attributs.include?(attribut)
                  end
                  changed_methods
                end
              end
            end
          end
        end

2/ In my concern or model, I had this to know the methods I need to "refresh" depending on changed attributes:

     def es_changed_methods
      {
        "last_connection_at" => [:es_connected_until, :es_visible_until],
        "lat" => [:es_location],
        "lng" => [:es_location]
      }
    end

last_connection_at, lat, png are my model's attributes
es_connected_until, es_visible_until, es_location are methods for as_indexed_json:

    def as_indexed_json options={}
      as_json only: ['id',
        'banned',
        'birthday',
        'certified',
        'confirmed_at',
        'name',
        'gender',
        'ghost_mode',
        'hide_age',
        'is_banned_by_admin', 
        'last_connection_at',
        'city'],
        methods: ['es_connected_until', 'es_friend_ids', 'es_location', 'es_profil_photo_url', 'es_visible_until']
    end 

Works perfectly!

@karmi
Copy link
Contributor

karmi commented Apr 16, 2014

@alotela You shouldn't override update_document in your code. Period. (At least override it for the proxy object in the specific model, not for all.)

My problem is that when I use methods array in as_indexed_json, they are not updated.

Yes, see #5 for a discussion about that. Just implement your own hooks for indexing, that's it, really.

@alotela
Copy link
Author

alotela commented Apr 16, 2014

I totally agree that overriding update_document is not the right way to do.
But:
1/ It is easy to use in all models... Just need to define the "es_changed_methods"
2/ maybe it would be a good solution to put directly into elastic search-rails so no more "override"
3/ I think solutions in #5 are more complicated and not really an easy way to do things, even if I saw your comment
"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?"

I personally think this problem should be managed by the gem easily... And defining a method to match changed_attributes and "changed_methods" looks good for me. It allows the gem to update only changed data.

What do you think ? ;)

@karmi
Copy link
Contributor

karmi commented Apr 16, 2014

I still think the cleanest way is to simply define your own indexing routines, in concert with your own serialization routines. The default callbacks are just that -- defaults for convenience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants