samedhi / firemore

Firebase + Clojure -> Firemore
https://firemore.org/
MIT License
19 stars 4 forks source link

Return value of "change" functions #35

Closed samedhi closed 4 years ago

samedhi commented 4 years ago

Currently, change functions (write!), (add!), (merge!), (delete!) return a channel. A value will be placed upon the channel IFF an error occurs, otherwise the channel is closed. This lets you write (if-let or (then-let), etc that only execute on failure.

The real outsider here is (add!). The problem with(add!)` is that I often need to know the id of the newly persisted document. This means I have to put a value (the id) on the channel on success. This breaks the whole "writes that succeed return a closed channel" philosophy.

I'll think on this some more.

samedhi commented 4 years ago

I am kind of thinking that you just should not support (add!) as a feature. It is trivial to generate a guid as an :id for a document. The only reason I can think you might need this feature is if you want to generate ids that cannot be in conflict (must be new) for some security reason?

samedhi commented 4 years ago

I don't like the (write!) vs (add!) difference.

write! is used to write a document to firestore while specifying the id

add! is used to write a document to firestore and let firestore generate the id for you.

My current thinking is that maybe there should only be a (write!), and that it should decide to use or create the id depending on whether you specify a reference to a id or a collection to these functions. I'm going to think about this before committing to anything.

samedhi commented 4 years ago

Today I am thinking that maybe all "write" functions should return a channel. The behavior is that the persisted document (including the :id) will be put on the channel OR the error map (a map that contains :firemore/error key) will be put on the channel. The channel will then be closed. This would mean that you can expect to always receive a value on the channel; the current behavior is to only get a value in the event of an error.

So now you can't do this

(when-let [error (<! (write! ...))]
  <handle the error>)

But you can still do this to "wait" for something to actually sync (or error) on the server

(<! (write! ...))
samedhi commented 4 years ago

Another possibility is to rather than putting a map on the thing to instead put a two-tuple of (doc, error), where one of the two items in the two-tuple would be nil.

(let [[doc error] (<! (write! ...)))]
  (if doc
    (println "This was successfully written => " doc)
    (println "Ugh oh => " error)))

So you can wait using (<! if all you want is to know if it completed (but don't care if it failed).

samedhi commented 4 years ago

I am closing this issue. Yes, it is annoying that add-db!, unlike every other write operation, does not return only on error but instead returns with the id. No, I am not going to change the contract for everything just to make add-db!'s behavior consistent with the rest.