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

Weird ActiveRel #1480

Closed kopylovvlad closed 6 years ago

kopylovvlad commented 6 years ago

I like neo4jrb, it is cool thing. But ActiveRel has some strange behavior.

I have 3 classes (2 ActiveNode and 1 ActiveRel) like this:

class User
  include Neo4j::ActiveNode
  include Neo4j::Timestamps
  include Neo4j::Timestamps::Created
  include Neo4j::Timestamps::Updated

  id_property :neo_id
  has_many :out, :quizzes, rel_class: :VotedFor
end

class Quiz
  include Neo4j::ActiveNode
  include Neo4j::Timestamps
  include Neo4j::Timestamps::Created
  include Neo4j::Timestamps::Updated

  id_property :neo_id
  has_many :in, :users, rel_class: :VotedFor
end

class VotedFor
  include Neo4j::ActiveRel
  include Neo4j::Timestamps
  include Neo4j::Timestamps::Created
  include Neo4j::Timestamps::Updated

  from_class :User
  to_class   :Quiz

  property :option_number, type: Integer
end

Let's create instances

Quiz.destroy_all
Quiz.create!(
  title: 'active quiz',
  body: 'just body',
  active: true,
  options: [
    { title: 'yes' },
    { title: 'no' }
  ]
)

user = User.first
quiz = Quiz.first
VotedFor.all.count # 0

rel = VotedFor.create!(
  from_node: user,
  to_node: quiz,
  option_number: 0
)

VotedFor.all.count # 1

Alright, it works pretty well. Let's try to find an instance of VotedFor class.

Why .where does not support searching by from_class, from_class? It will be awesome feature.

VotedFor.where(from_class: user) # it always returns []
VotedFor.where(to_class: quiz) # it always returns []

Why .where does not work with id?

VotedFor.where(id: rel.id) # it always returns []

Methods .find and .find_by_id work perfectly, but...

VotedFor.find(rel.id)
VotedFor.find_by_id(rel.id)

Let's delete an instance of VotedFor

rel_id = rel.id
rel.destroy
VotedFor.all.count # 0

And after that, we will try to find our instance by .find and .find_by_id

VotedFor.find(rel_id) # NoMethodError: undefined method `r' for nil:NilClass
VotedFor.find_by_id(rel.id) # NoMethodError: undefined method `r' for nil:NilClass

Why does it raise an NoMethodError? Maybe, the better way will be raise something like Neo4j::ActiveNode::Labels::RecordNotFound?

For example: ActiveNode works perfectly and can raise RecordNotFound like rails/activerecord.

Quiz.find(99999)
# Neo4j::ActiveNode::Labels::RecordNotFound: Couldn't find Quiz with 'neo_id'=99999

Runtime information:

Neo4j database version: neo4j gem version: 9.1.3 neo4j-core gem version: 8.1.0

cheerfulstoic commented 6 years ago

We're actually in the process of phasing out finder methods for ActiveRel. Neo4j encourages finding things by nodes first and then traversing relationships. There is no performant way (at least in Cypher) to find a relationship by an ID. So instead of saying:

VotedFor.where(from_class: user)
user.quizzes.each_rel
# Or to just get one:
user.quizzes.each_rel.first

Does that make sense? That should probably be clearer in the docs

FYI there's no need to include Neo4j::Timestamps::Created or Neo4j::Timestamps::Updated when you've included Neo4j::Timestamps. Neo4j::Timestamps will bring in both for you.