Fork me on GitHub
#duct
<
2019-11-18
>
paulspencerwilliams07:11:58

So…. I was following this excellent second post in a series on teaching Duct where @agile_geek switched databases from Sqlite to Postgresql. This was nearly okay, except when trying to create films, I receive the error

Film not added due to ERROR: column "rating" is of type integer but expression is of type character varying Hint: You will need to rewrite or cast the expression. Position: 65
This is because ratings which should be an integer are coming through as strings from the http interface, and need to be cast to integers somewhere through the Duct pipeline. But where? The boundary?
(extend-protocol FilmDatabase duct.database.sql.Boundary
  (list-films [{db :spec}]
    (jdbc/query db ["select * from film"]))
  (create-film [{db :spec} film]
    (try
      (let [result (jdbc/insert! db :film film)]
        (if-let [id (val (ffirst result))]
          {:id id}
          {:errors ["Failed to add film"]}))
      (catch SQLException ex
        {:errors [(format "Film not added due to %s" (.getMessage ex))]}))))
(defmethod ig/init-key :film-ratings.handler.film/create [_ {:keys [db]}]
  (fn [{[_ film-form] :ataraxy/result :as request}]
    (let [film (reduce-kv
                 (fn [m k v] (assoc m (keyword k) v))
                 {}
                 (dissoc film-form "__anti-forgery-token"))
          result (boundary.film/create-film db film)
          alerts (if (:id result)
                   {:messages ["Film added"]}
                   result)]
      [::response/ok (views.film/film-view film alerts)])))
Neither seem ideal? :thinking_face: https://circleci.com/blog/package-a-clojure-web-application-using-docker/ My repo, although I’ve not pushed the offending commit to support Postgresql yet as it’s not strictly needed to explain my problem?: https://github.com/paulspencerwilliams/film-rating

rickmoynihan09:11:29

Not looked at the details of your example, but it sounds like you want to look at ataraxy coercers.

paulspencerwilliams09:11:30

Ooh, they sound like what I’d want! Cheers’

paulspencerwilliams10:11:13

There’s some documentation on coercing route params etc, but I’ve not been able to find any on coercing form maps :thinking_face:

rickmoynihan10:11:03

ahh yeah I don’t think you can do form params with them

rickmoynihan11:11:04

I’m not a huge fan of ataraxy tbh… We tend to coerce the ataraxy request object into a context map at the top of most handlers, and do initial massaging/coercion beyond the coercers there. In particular converting the annoyingly always positional arguments of :ataraxy/result into a map.

paulspencerwilliams11:11:34

Okay, yeah, that was what I thinking, taking control of the data. Do you have public examples of this pattern?

paulspencerwilliams11:11:43

Cheers for the advice btw!

rickmoynihan11:11:24

Sorry. We I don’t think we have any duct projects in public repos at the minute…

rickmoynihan11:11:39

but cribbed and abridged from real code… this sort of thing is pretty typical for us:

rickmoynihan11:11:40

(defn build-context [opts {[_ arg-a arg-b] :ataraxy/result :as request}]
  (-> opts
      (assoc :request request
             :arg/a arg-a
             :arg/b arg-b)))

(defn some-handler [{:keys [layout] :as opts} request]
  (let [ctx (build-context opts request)
        view-model (query-db ctx)]
    (if view-model
      [:ataraxy.response/ok (str (h/html
                                  (layout (render ctx view-model))))]

      [:ataraxy.response/not-found "Not found"])))

(defmethod ig/init-key :app/some-handler [_ opts]
  (partial some-handler opts))

weavejester12:11:17

A coercion library like spec-coerce might also be worth looking into.

weavejester12:11:49

Generally I think it’s a good idea to coerce as soon as possible, so at the handler level.

rickmoynihan12:11:16

also meant to say earlier the advantage of doing it in your code is that you can stick a spec on it more easily on a route by route basis.

rickmoynihan12:11:39

so yeah spec-coerce might be worth looking at… though i’ve not used it myself

weavejester12:11:48

With regard to Ataraxy’s positional arguments, I think I’d be open to a PR that supports maps. I’d also be very open to adding alternative routers to Duct. There’s quite a few around.

rickmoynihan12:11:31

Yeah maps would typically be much more useful than positional args in ataraxy — though I know it needs to be positional because query params aren’t strictly keys/values they have an order too… it’s more just seldom used/required.

rickmoynihan12:11:51

Regarding other routers we looked at switching to reitit… I don’t think there were significant blockers; though I think we’d need to check that the config had semantics that worked under meta-merge

weavejester12:11:40

spec-tools also has coercion, so that might be worth looking at, too.

paulspencerwilliams12:11:46

Oh, I stepped away for lunch and missed the conversation!

paulspencerwilliams12:11:03

I was thinking how spec could be involved…

weavejester12:11:29

You didn’t miss much - I was just suggesting that there are coercion libraries that you might want to take a look at.

paulspencerwilliams12:11:41

cheers for the advice!

paulspencerwilliams19:11:18

In the end, I’ve used spec-tools to coerce the data, and will add validation later. https://github.com/paulspencerwilliams/film-rating/commit/c2bcc4828915df9f9566a5c72712780c4da15ef8 Cheers, again for the pointer.

rickmoynihan18:11:03

Just run into an issue with returning ataraxy responses e.g. [::resp/ok ,,,] which is that their use can prevent you mixing standard ring middlewares onto specific routes. I’ve never entirely understood the reason to use these responses, rather than the raw ring maps. Can anyone clarify?

weavejester00:11:49

Their use allows more sophisticated handling of certain types of exceptions, particularly around error messages.

weavejester00:11:17

For example, the responses ::missing-params and ::failed-spec both produce a 400 “bad request” response. But because Ataraxy distinguishes them internally, you can dispatch off an unambiguous key.

weavejester00:11:51

With regard to mixing in middleware, the middleware should be added after the response is resolved. If it’s not doing that, then that’s a bug!

rickmoynihan10:11:36

Yeah fair point about extending new codes via the multi-methods… Ok could be a bug then… I think this is a minimal failing case:

((ataraxy.core/handler {
     :routes '{[:get "/hello"] ^:af [:hello]} 
     :middleware {:af ring.middleware.anti-forgery/wrap-anti-forgery} 
     :handlers {:hello (fn [req] [:atraxy.response/ok "hello"])}}) 

{:uri "/hello" :request-method :get})
Which raises:
1. Unhandled java.lang.IllegalArgumentException
   Key must be integer

    APersistentVector.java:  347  clojure.lang.APersistentVector/assoc
    APersistentVector.java:   18  clojure.lang.APersistentVector/assoc
                   RT.java:  827  clojure.lang.RT/assoc
                  core.clj:  191  clojure.core/assoc
                  core.clj:  190  clojure.core/assoc
               session.clj:   25  ring.middleware.anti-forgery.session.SessionStrategy/write_token
          anti_forgery.clj:   95  ring.middleware.anti-forgery/wrap-anti-forgery/fn
                  core.clj:  307  ataraxy.core/apply-handler
                  core.clj:  304  ataraxy.core/apply-handler
                  core.clj:  340  ataraxy.core/handler/fn
                      REPL: 8278  dev/eval98143
                      REPL: 8278  dev/eval98143
             Compiler.java: 7177  clojure.lang.Compiler/eval
             Compiler.java: 7132  clojure.lang.Compiler/eval
                  core.clj: 3214  clojure.core/eval
                  core.clj: 3210  clojure.core/eval
                  main.clj:  437  clojure.main/repl/read-eval-print/fn
                  main.clj:  437  clojure.main/repl/read-eval-print
                  main.clj:  458  clojure.main/repl/fn
                  main.clj:  458  clojure.main/repl
                  main.clj:  368  clojure.main/repl
               RestFn.java: 1523  clojure.lang.RestFn/invoke
    interruptible_eval.clj:   79  nrepl.middleware.interruptible-eval/evaluate
    interruptible_eval.clj:   55  nrepl.middleware.interruptible-eval/evaluate
    interruptible_eval.clj:  142  nrepl.middleware.interruptible-eval/interruptible-eval/fn/fn
                  AFn.java:   22  clojure.lang.AFn/run
               session.clj:  171  nrepl.middleware.session/session-exec/main-loop/fn
               session.clj:  170  nrepl.middleware.session/session-exec/main-loop
                  AFn.java:   22  clojure.lang.AFn/run
               Thread.java:  748  java.lang.Thread/run
Also I went to file this as an issue, and it looks like it’s already there dating back to 2017: https://github.com/weavejester/ataraxy/issues/11 Will post the above on that issue too.

weavejester18:11:17

I haven’t had a lot of time to spare on Ataraxy recently, so it might take me a while to take a look at the middleware issue.

weavejester18:11:22

I may just write a few quick Duct integrations for other routing libraries so people will at least be able to choose. I still use Ataraxy myself, but I recognize that it has rough edges that I haven’t been able to sand down.