neo4jrb / activegraph

An active model wrapper for the Neo4j Graph Database for Ruby.
http://neo4jrb.io
MIT License
1.4k stars 276 forks source link

'after_find' callback is not being called as expected #1538

Open adamlwatson opened 5 years ago

adamlwatson commented 5 years ago

As the title states, the after_find ActiveNode callback does not get executed after performing a query that returns a record.

Additional information which could be helpful if relevant to your issue:

Code example (inline, gist, or repo)

Test code:

class Testmodel
  include Neo4j::ActiveNode
  property :name

  after_initialize :after_initialize
  after_find :after_find

  def after_initialize
    puts ("In after_initialize callback")
  end

  def after_find
    puts ("In after_find callback")
  end
end
[1] pry(main)> n = Testmodel.create(name: 'test')
In after_initialize callback
=> #<Testmodel uuid: "a4808485-6a45-41ce-82ec-76874ab0b2ea", name: "test">
[2] pry(main)> Testmodel.find_by name: 'test'
In after_initialize callback
=> #<Testmodel uuid: "ed26036a-19e3-41c3-ad36-8a45e34156e8", name: "test">

As you can see from the above test code, after_initialize is called as expected, but after_find is not.

Digging through the source, I can see a comment in neo4j-9.4.0/lib/neo4j/shared/callbacks.rb on line 12 that states:

# after_find is triggered by the `find` method defined in lib/neo4j/active_node/id_property.rb

But when I look at neo4j-9.4.0/lib/neo4j/active_node/id_property.rb, it does not look as if any of the find* methods are calling the after_find callback once their logic is executed.

Runtime information:

Neo4j database version: neo4j gem version: 9.4.0 neo4j-core gem version: 9.0.0

OpenCoderX commented 5 years ago

Looks like it is only called when finding via the Model.find method. https://github.com/neo4jrb/neo4j/blob/f1c3a5bb29d92bbcbe3c948a43f23b35020dc467/lib/neo4j/active_node/labels.rb#L107

adamlwatson commented 5 years ago

Thanks for the reply... looks like you are correct. This seems broken... wondering why the callback wouldn’t be implemented for all find methods. If it’s a matter of not wanting to evaluate queries prematurely, some sort of hint method, or a standard enumerable call could be added to specify that evaluation should occur.

Anyway, definitely worth a bit of extra documentation to avoid confusion.

OpenCoderX commented 5 years ago

It looks like it would be fairly straight forward to make the callbacks fire for find_by, I noticed that find_by only return the first match, is that preferred?

def find_by(values)
    all.where(values).limit(1).query_as(:n).pluck(:n).first
 end
deobald commented 4 years ago

The complete list of methods which should fire the after_find callbacks: