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

Data loss in p/add with nested maps #14

Closed
smichal opened this issue Dec 13, 2021 · 2 comments
Closed

Data loss in p/add with nested maps #14

smichal opened this issue Dec 13, 2021 · 2 comments

Comments

@smichal
Copy link

smichal commented Dec 13, 2021

Hi,
I've noticed that in some cases pyramid normalisation replaces an entity with a reference but losses data of this entity.
Example:

(p/add {} {:a/id 1 :b [{:c {:d/id 1 :d/txt "a"}}]})
=>  #:a{:id
      {1 {:a/id 1, :b [{:c [:d/id 1]}]}}}
@lilactown
Copy link
Owner

I can reproduce this. The issue is dual pronged, starting with this line in the normalize function

https://github.com/lilactown/pyramid/blob/main/src/pyramid/core.cljc#L93-L94

basically, normalization doesn't descend into collections of maps where the maps are not entities. The map inside of the vector doesn't contain a key whose name is id, which is why it's not considered an entity. This means normalization misses the entity identified by :d/id.

However, we do a second pass to replace entities with their references. This does descend into collections of maps even if they are not entities

https://github.com/lilactown/pyramid/blob/main/src/pyramid/core.cljc#L70-L71

I think what I'd like to do is replace these two functions with a single pass process that both adds entities to the db and replaces them with references, so that we can ensure the way that each collection is visited is the same so that we don't run into issues like this where we lose data.

Thank you for the report! I've added a failing test here: 972eceb

@lilactown
Copy link
Owner

Fixed in #19

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

Successfully merging a pull request may close this issue.

2 participants