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

Creating relations with ActiveGraph::Relationship may delete other existing :has_one relations from the involved nodes #1650

Open
wlaoman opened this issue Mar 18, 2021 · 1 comment

Comments

@wlaoman
Copy link

wlaoman commented Mar 18, 2021

The problem occurs when using the same relation class for both :in and :out relations.

I am building a tree structure using ActiveGraph::Nodes, and use ActiveGraph::Relationship for the relations between the nodes (to store some information in the relations).

class NodeWithRel 
  include ActiveGraph::Node
  
  has_one  :out, :parent,   rel_class: :NodeRel
  has_many :in,  :subnodes, rel_class: :NodeRel
  
  property :title
end

class NodeRel 
  include ActiveGraph::Relationship
  
  from_class :NodeWithRel
  to_class   :NodeWithRel
end

When I add a subnode to an existing node, the existing node's parent relation is deleted when it is saved to the database:

irb(main):001:1* n = NodeWithRel.new(title: "root")
=> #<NodeWithRel uuid: nil, title: "root">
irb(main):002:0> n.save
=> true
irb(main):003:0> n1 = NodeWithRel.new(title: "n1")
=> #<NodeWithRel uuid: nil, title: "n1">
irb(main):004:0> n.subnodes << n1
=> #<QueryProxy NodeWithRel#subnodes [#<NodeWithRel uuid: "d66f8b32-961c-4113-9673-3e67c1f14973", title: "n1">]>
irb(main):005:0> n1.parent
=> #<NodeWithRel uuid: "ba965b32-b0a3-444f-b872-8eb0c9912b91", title: "root">
irb(main):006:0> n1_1 = NodeWithRel.new(title: "n1_1")
=> #<NodeWithRel uuid: nil, title: "n1_1">
irb(main):007:0> n1.subnodes << n1_1
=> #<QueryProxy NodeWithRel#subnodes [#<NodeWithRel uuid: "3874ae42-6379-4d8c-9580-0faf75ce8c88", title: "n1_1">]>
irb(main):008:0> n1.parent
=> nil # <-- should be the first node created, with the title "root"

This doesn't happen if I use "ordinary" relations instead of ActiveGraph::Relationship:

class Node 
  include ActiveGraph::Node
  
  has_one  :out, :parent,   type:   :parent,  model_class: :Node
  has_many :in,  :subnodes, origin: :parent,  model_class: :Node
  
  property :title
end

irb(main):010:0> m = Node.new(title: "root")
=> #<Node uuid: nil, title: "root">
irb(main):011:0> m.save
=> true
irb(main):012:0> m1 = Node.new(title: "m1")
=> #<Node uuid: nil, title: "m1">
irb(main):013:0> m.subnodes << m1
=> #<QueryProxy Node#subnodes [#<Node uuid: "46975b28-9f3b-4edf-947f-87a6f3428be8", title: "m1">]>
irb(main):014:0> m1.parent
=> #<Node uuid: "c6bc7943-3186-46f2-ad6a-8aa804b450a1", title: "root">
irb(main):015:0> m1_1 = Node.new(title: "m1_1")
=> #<Node uuid: nil, title: "m1_1">
irb(main):016:0> m1.subnodes << m1_1
=> #<QueryProxy Node#subnodes [#<Node uuid: "531ee78a-603f-47b0-be2b-bfd1581a0bf7", title: "m1_1">]>
irb(main):017:0> m1.parent
=> #<Node uuid: "c6bc7943-3186-46f2-ad6a-8aa804b450a1", title: "root">

I think the reason might be that when the relation is set up, ActiveGraph::Relationship first deletes any existing relations that are :has_one relation.
In active_graph/relationship/persistance.rb, line 55:

    def delete_has_one_rel
      to_node.delete_reverse_has_one_relationship(self, :in, from_node) if to_node.persisted?
      from_node.delete_reverse_has_one_relationship(self, :out, to_node) if from_node.persisted?
    end

In active_graph/node/has_n.rb, line 244:

    def delete_reverse_has_one_relationship(relationship, direction, other_node)
      rel = relationship_corresponding_rel(relationship, direction, other_node.class)
      delete_has_one_rel!(rel.last) if rel && rel.last.type == :has_one
    end

and line 254:

    def relationship_corresponding_rel(relationship, direction, target_class)
      self.class.associations.find do |_key, assoc|
        assoc.relationship_class_name == relationship.class.name ||
          (assoc.relationship_type == relationship.type.to_sym && assoc.target_class == target_class && assoc.direction == direction)
      end
    end

When using ActiveGraph::Relationship, #relationship_corresponding_rel doesn't consider the direction of the relation when selecting, so when there are two relations using the same relationship class (as in my tree example), both relationships are returned, and one of them (the last) is deleted by #delete_reverse_has_one_relationship.

Changing line 256 from:
assoc.relationship_class_name == relationship.class.name ||
to:
(assoc.relationship_class_name == relationship.class.name && assoc.direction == direction) ||
does fix the problem for me, but I am not sure if this change has any other side effects.

Runtime information:

Neo4j database version: 4.0.11
activegraph gem version: 10.0.2
neo4j-ruby-driver gem version: 1.7.4

@amitsuryavanshi
Copy link
Member

@wlaoman I do not see any side effects of this change. Thanks for looking into this. We would release the fix in next version.

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