Replacing an entity using `delete` and `add` throws exception if entity does not exist
wardle opened this issue · 2 comments
When there is already data in the database, this works:
(-> {}
(pyr/add {:person/id 2 :person/name "Mark"})
(pyr/delete [:person/id 2])
(pyr/add {:person/id 2 :person/name "Will"}))
=> #:person{:id {2 #:person{:id 2, :name "Will"}}}
If one avoids the delete, there is also no issue:
(-> {}
(pyr/add {:person/id 2 :person/name "Mark"})
#_(pyr/delete [:person/id 2])
(pyr/add {:person/id 2 :person/name "Will"}))
=> #:person{:id {2 #:person{:id 2, :name "Will"}}}
But if there is no initial data in the database at all for that entity:
(-> {}
#_(pyr/add {:person/id 2 :person/name "Mark"})
(pyr/delete [:person/id 2])
(pyr/add {:person/id 2 :person/name "Will"}))
Execution error (NullPointerException) at pyramid.core/update-ref (core.cljc:85).
Cannot invoke "clojure.lang.Associative.assoc(Object, Object)" because "em" is null
Of course, it is fine if there is at least one other entity in the database:
(-> {}
(pyr/add {:person/id 1 :person/name "Mark"})
(pyr/delete [:person/id 2])
(pyr/add {:person/id 2 :person/name "Will"}))
=> #:person{:id {1 #:person{:id 1, :name "Mark"}, 2 #:person{:id 2, :name "Will"}}}
Looking at the code, it looks as if this exception could never be thrown, as em
in update-ref
should never be nil, as the get falls back to a {}. The issue I think is that delete clears the value to nil, but the key exists, so the get
returns nil and not an empty map.
One fix is I think to replace (get m k {})
with (or (get m k) {})
.
I will create a fork and submit a pull request if you are think that would be acceptable?
The fundamental issue is that delete results in a nil value:
(-> {}
(p/delete [:person/id 1]))
=> #:person{:id nil}
But my inclination is that it is better to not touch the delete code so it handles this edge case, and instead fix update-ref
to operate with a nil
value. I'll submit the PR on this basis but you might prefer a different solution of course!