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

Bug with scopes/class methods and `searchkick` gem #1208

Open ProGM opened 8 years ago

ProGM commented 8 years ago

I was following the searchkick manual. https://github.com/ankane/searchkick#indexing

As written, I defined a search_import scope to define eager loaded values for custom indexed data.

I tried both defining a scope (as stated in the guide) and a class method. Both raises an error when I run reindex:

class Article
  scope :search_import, -> { all.with_associations(:published_version) }
end
class Article
  def self.search_import
    all.with_associations(:published_version)
  end
end

They both raises NoMethodError: undefined method 'identity' for #<#<Class:0x007f961a2a97b8>:0x007f961a2a8520>

Runtime information:

Neo4j database version: neo4j gem version: 7.0.6+ (master HEAD) neo4j-core gem version: 6

cheerfulstoic commented 8 years ago

I think I've fixed it. Just released patch versions: 6.0.9, 6.1.12, and 7.0.8. Give that a try! ;)

ProGM commented 8 years ago

Wow, it was really a code bug!

I tested the patch. Now I get no error, but the eager load is not working...

class Article
  include Neo4j::ActiveNode
  include Neo4j::Timestamps
  include Searchkick::Model
  searchkick
  def self.search_import
    all.with_associations(:published_version)
  end

  def search_data
    attributes.merge(
      'published' => !published_version.nil?
    )
  end
end
irb(main):016:0> Article.reindex
 CYPHER 19ms MATCH (n:`Article`) RETURN n ORDER BY n.uuid LIMIT {limit_1000} | {:limit_1000=>1000}
 Article#published_version 4ms MATCH (article516) WHERE (ID(article516) = {ID_article516}) MATCH (article516)-[rel1:`published_version`]->(result_published_version:`Version`:`Quotable`) RETURN result_published_version | {:ID_article516=>516}
 Article#published_version 2ms MATCH (article101) WHERE (ID(article101) = {ID_article101}) MATCH (article101)-[rel1:`published_version`]->(result_published_version:`Version`:`Quotable`) RETURN result_published_version | {:ID_article101=>101}
 Article#published_version 6ms MATCH (article74) WHERE (ID(article74) = {ID_article74}) MATCH (article74)-[rel1:`published_version`]->(result_published_version:`Version`:`Quotable`) RETURN result_published_version | {:ID_article74=>74}
 Article#published_version 5ms MATCH (article131) WHERE (ID(article131) = {ID_article131}) MATCH (article131)-[rel1:`published_version`]->(result_published_version:`Version`:`Quotable`) RETURN result_published_version | {:ID_article131=>131}

... MANY OTHERS...

 Article#published_version 4ms MATCH (article80) WHERE (ID(article80) = {ID_article80}) MATCH (article80)-[rel1:`published_version`]->(result_published_version:`Version`:`Quotable`) RETURN result_published_version | {:ID_article80=>80}
  Article Import (280.1ms)  {"count":38}
D, [2016-05-27T10:53:52.457240 #56982] DEBUG -- :   Article Import (280.1ms)  {"count":38}
=> true
cheerfulstoic commented 8 years ago

I think it comes down to this:

https://github.com/ankane/searchkick/blob/master/lib/searchkick/model.rb#L53

It's calling searchkick_klass which is going to get Article and so even if you do Article.search_import.reindex (I assume that's what you meant to do) it's just going to use the base class.

I think there could be a couple of ways of fixing this, depending on how general you want the solution to be. It looks like the reindex method is simply aliased to searchkick_reindex if reindex doesn't already exist. Thus you could write your own reindex for your model and call searchkick_reindex` yourself (I suspect that's what the aliasing is all about).

Alternatively you could define a searchkick_klass method to overwrite the class reader here:

https://github.com/ankane/searchkick/blob/master/lib/searchkick/model.rb#L11

You could even define that on QueryProxy (thus defining it on AssociationProxy since AssociationProxy defers all missing methods to QueryProxy) so that it could simply just work anywhere. If that work, it might be a simple gemneo4j-elasticsearchfor anybody who wants to have the same hack. Probably theREADMEcould explain thatsearchkick` works with Neo4j normally but if you want to do this special stuff you would need the gem (and maybe in the future the gem might contain other helper code)