QueryProxy replace_with compares IDs globally, instead of per model-class

[yourpalal]

While investigating #1652, I noticed that QueryProxy replace_with compares IDs for all models to determine which relationships to delete and insert.

Assuming the id in question is a uuid, this probably works out fine, but when using multiple model classes, there could be some overlap of ids between the two model classes.

Note: the problem in the example below is currently hidden by the bug reported in #1652 . If pluck(:id) is were fixed, but no other changes were applied, then #1652 could be considered fixed, while this issue would not be.

Probably the best course of action is to fix this and #1652 at the same time.

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_zine, book = [ Magazine.create!(issue_id: "1234"), Book.create!(isbn: "098512=25") ]
publisher = Publisher.create!
publisher.published_things = [original_zine, book]

replacement_zine = Magazine.create!(issue_id: "098512=25") # same as book isbn
publisher.published_things = [replacement_zine] # leads to replace_with being called

# replacement_zine did not get a rel, since it has the same id as book
replacement_zine.reload
puts replacement.publisher # puts nil

# book was not removed, since it has the same id as replacement_zine
book.reload
puts book.publisher # puts non-nil

Runtime information:

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

[klobuczek]

@yourpalal do you currently have overlapping ids between models? I know that requiring global uniqueness is unnecessarily strong, but it is best practice. What id generator are you using?

[yourpalal]

We do have some overlapping ids between models. We import various external datasets and use the IDs used in those datasets as id_properties. Often, one data set will have several data types, which then become different models in activegraph.

In some cases, there is overlap of IDs within a single dataset, and in others there is overlap between these datasets.

Many datasets we import use integer ids (probably AUTOINT in their database) which can increase the likelihood of overlap between datasets.

---

Generally, any non-uuid ID property could end up running into this issue. If universally-unique IDs are required by ActiveGraph, that would be pretty easy to achieve by always using a uuid property, but I don't believe this is stated anywhere in the documentation.