threatgrid / asami

A graph store for Clojure and ClojureScript
Eclipse Public License 1.0
637 stars 29 forks source link

Transaction append annotation fails on IPersistentMap #220

Open kyleerhabor opened 3 years ago

kyleerhabor commented 3 years ago

It seems that a transaction using the + append annotation will fail when a map is supplied. For reference, I'm using Asami 2.2.1 and Clojure 1.10.3.855.

(def conn (d/connect "asami:local://demo"))
#'hue.server.core/conn

(d/transact conn [{:title "Demo"
                   :description "This is a demonstration of the append annotation failing on maps."
                   :contents [{:statement "Wow"}]}])
#object[java.util.concurrent.CompletableFuture 0x4e0964bb "java.util.concurrent.CompletableFuture@4e0964bb[Completed normally]"]

(d/entity (d/db conn) (asami.graph/new-node 1))
{:title "Demo",
 :description "This is a demonstration of the append annotation failing on maps.",
 :contents [{:statement "Wow"}]}

(d/transact conn [{:db/id (asami.graph/new-node 1)
                   :contents+ {:statement "What?"}}])
; Execution error (IllegalArgumentException) at asami.durable.encoder/eval22954$fn$G (encoder.clj:107).
; No implementation of method: :encapsulate-id of protocol: #'asami.durable.encoder/FlatFile found for class: clojure.lang.PersistentArrayMap

As FlatFile does not implement the encapsulate-id function for IPersistentMap, this error seems like expected behavior (or temporary). https://github.com/threatgrid/asami/blob/d3b67520a46f8a98e244050aed09e44e577bcb8b/src/asami/durable/encoder.clj#L333-L341

If it's the case, how can I append a map to a vector? I could use the ' replacement annotation and set the vector as its previous items with the new one appended, but it seems inefficient and unintended.

quoll commented 2 years ago

IPersistentMap was the wrong thing to have here, and it's been fixed, but it's not the cause of this problem. In this case, there appears to be a missing code path for adding a map to the sequence (unless it's to be saved as a map object, but this isn't what the API is supposed to do here).

The latest version will actually save this map as a "value" in the list, which isn't consistent either. I'll see if I can track down where this is happening in the entity writer code.