activegraph icon indicating copy to clipboard operation
activegraph copied to clipboard

QueryProxy replace_with + multiple model classes + id_property = nothing removed

Open yourpalal opened this issue 3 years ago • 1 comments

When defining a has_many relationship with more than one model classes, and the model classes have defined custom id properties, QueryProxy replace_with fails to correctly delete the old relationships.

This is because QueryProxyLink#converted_key converts :id to :uuid when no model class is given. Due to this issue,pluck(:id) returns an array of nils in this scenario.

Suggested actions:

  • refactor replace_with to get the right ids for the various model classes
  • change converted_key to raise an error in this situation, instead of returning uuid. This may cause some breakage, but it will change some already-broken code from failing silently to failing with an exception, which is an improvement in my opinion.

Code example (inline, gist, or repo)

class Book
  include ActiveGraph::Node
  
  id_property :isbn
  
  has_one :in, :publisher, origin: :published_things, model_class: %w(Publisher)
end

class Magazine
  include ActiveGraph::Node
  
  id_property :issue_id
  
  has_one :in, :publisher, origin: :published_things, model_class: %w(Publisher)
end

class Publisher
  include ActiveGraph::Node
  
  has_many :out, :published_things, type: :published, model_class: %w(Book Magazine)
end

original = [ Magazine.create!(issue_id: "1234"), Book.create!(isbn: "098512=25") ]
publisher = Publisher.create!
publisher.published_things = original

replacement = [ Magazine.create!(issue_id: "09820685") ]
publisher.published_things = replacement # leads to replace_with being called
publisher.reload

puts publisher.published_things.count # puts 3

note: I ran into this problem while also using an ActiveGraph::Relationship. I'm pretty sure that's not relevant to the issue, but I could be wrong.

Runtime information:

Neo4j database version: neo4j gem version: 10.1.0 neo4j-ruby-driver gem version: 1.7.5

yourpalal avatar Apr 05 '21 18:04 yourpalal

Another alternative here would be to change replace_with to always just fully delete before inserting the new relationships.

yourpalal avatar Apr 05 '21 18:04 yourpalal