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

changed assertion Error to Exception issue #49 #142

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions project.clj
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
(defproject yesql "0.5.3"
(defproject org.batch/yesql "0.7.3"
:description "A Clojure library for using SQL"
:url "https://github.com/krisajenkins/yesql"
:license {:name "Eclipse Public License"
:url "http://www.eclipse.org/legal/epl-v10.html"}
:dependencies [[org.clojure/clojure "1.8.0"]
[org.clojure/java.jdbc "0.5.8"]
[instaparse "1.4.1" :exclusions [org.clojure/clojure]]]
[org.clojure/java.jdbc "0.7.12"]
[instaparse "1.4.1" :exclusions [org.clojure/clojure]]
[com.brunobonacci/safely "0.5.0-alpha8" :exclusions [com.google.errorprone/error_prone_annotations org.clojure/clojure com.google.code.findbugs/jsr305]]]
:pedantic? :abort
:scm {:name "git"
:url "https://github.com/krisajenkins/yesql"}
:profiles {:dev {:dependencies [[expectations "2.1.3" :exclusions [org.clojure/clojure]]
[org.apache.derby/derby "10.12.1.1"]]
:plugins [[lein-autoexpect "1.4.0"]
:plugins [[lein-autoexpect "1.4.0" :exclusions [org.clojure/tools.namespace]]
[lein-expectations "0.0.8" :exclusions [org.clojure/clojure]]]}
:1.5 {:dependencies [[org.clojure/clojure "1.5.1"]]}
:1.6 {:dependencies [[org.clojure/clojure "1.6.0"]]}
Expand Down
113 changes: 74 additions & 39 deletions src/yesql/generate.clj
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
(ns yesql.generate
(:require [clojure.java.jdbc :as jdbc]
[clojure.set :as set]
[clojure.string :refer [join lower-case]]
[clojure.string :refer [join lower-case split trim]]
[yesql.util :refer [create-root-var]]
[yesql.types :refer [map->Query]]
[yesql.statement-parser :refer [tokenize]])
(:import [yesql.types Query]))
[yesql.statement-parser :refer [tokenize]]
[safely.core :refer [safely]])
(:import yesql.types.Query)
(:import java.sql.SQLException)
(:import java.lang.IllegalArgumentException))

(def in-list-parameter?
"Check if a type triggers IN-list expansion."
Expand All @@ -14,7 +17,7 @@
(defn- args-to-placeholders
[args]
(if (in-list-parameter? args)
(clojure.string/join "," (repeat (count args) "?"))
(if (empty? args) "NULL" (clojure.string/join "," (repeat (count args) "?")))
"?"))

(defn- analyse-statement-tokens
Expand All @@ -35,37 +38,47 @@
(defn rewrite-query-for-jdbc
[tokens initial-args]
(let [{:keys [expected-keys expected-positional-count]} (analyse-statement-tokens tokens)
actual-keys (set (keys (dissoc initial-args :?)))
actual-keys (set (keys (dissoc (if (or (vector? initial-args) (list? initial-args)) (apply merge initial-args) initial-args) :?)))
actual-positional-count (count (:? initial-args))
missing-keys (set/difference expected-keys actual-keys)]
(assert (empty? missing-keys)
(format "Query argument mismatch.\nExpected keys: %s\nActual keys: %s\nMissing keys: %s"
(if-not (empty? missing-keys)
(throw (IllegalArgumentException. (format "Query argument mismatch.\nExpected keys: %s\nActual keys: %s\nMissing keys: %s"
(str (seq expected-keys))
(str (seq actual-keys))
(str (seq missing-keys))))
(assert (= expected-positional-count actual-positional-count)
(format (join "\n"
(str (seq missing-keys))))))
(if-not (= expected-positional-count actual-positional-count)
(throw (IllegalArgumentException. (format (join "\n"
["Query argument mismatch."
"Expected %d positional parameters. Got %d."
"Supply positional parameters as {:? [...]}"])
expected-positional-count actual-positional-count))
(let [[final-query final-parameters consumed-args]
(reduce (fn [[query parameters args] token]
(cond
(string? token) [(str query token)
parameters
args]
(symbol? token) (let [[arg new-args] (if (= '? token)
[(first (:? args)) (update-in args [:?] rest)]
[(get args (keyword token)) args])]
[(str query (args-to-placeholders arg))
(vec (if (in-list-parameter? arg)
(concat parameters arg)
(conj parameters arg)))
new-args])))
["" [] initial-args]
tokens)]
(concat [final-query] final-parameters))))
expected-positional-count actual-positional-count))))
(if (or (vector? initial-args) (list? initial-args))
(let [[final-query final-parameters consumed-args]
(reduce (fn [[query parameters args] token]
(cond
(string? token) [(str query token)
parameters
args]
(symbol? token) [(str query (args-to-placeholders ""))
(conj parameters (keyword token))
args])) ["" [] initial-args] tokens)] (concat [final-query] (mapv (apply juxt final-parameters) initial-args)))
(let [[final-query final-parameters consumed-args]
(reduce (fn [[query parameters args] token]
(cond
(string? token) [(str query token)
parameters
args]
(symbol? token) (let [[arg new-args] (if (= '? token)
[(first (:? args)) (update-in args [:?] rest)]
[(get args (keyword token)) args])]
[(str query (args-to-placeholders arg))
(vec (if (in-list-parameter? arg)
(concat parameters arg)
(conj parameters arg)))
new-args])))
["" [] initial-args]
tokens)]
(concat [final-query] final-parameters)))))

;; Maintainer's note: clojure.java.jdbc.execute! returns a list of
;; rowcounts, because it takes a list of parameter groups. In our
Expand All @@ -77,7 +90,9 @@

(defn insert-handler
[db statement-and-params call-options]
(jdbc/db-do-prepared-return-keys db statement-and-params))
(if (vector? (second statement-and-params))
(jdbc/execute! db statement-and-params {:multi? true :return-keys true})
(jdbc/db-do-prepared-return-keys db statement-and-params)))

(defn query-handler
[db sql-and-params
Expand Down Expand Up @@ -110,16 +125,36 @@
global-connection (:connection query-options)
tokens (tokenize statement)
real-fn (fn [args call-options]
(let [connection (or (:connection call-options)
global-connection)]
(assert connection
(format (join "\n"
["No database connection supplied to function '%s',"
"Check the docs, and supply {:connection ...} as an option to the function call, or globally to the defquery declaration."])
name))
(jdbc-fn connection
(rewrite-query-for-jdbc tokens args)
call-options)))
(if (:debug call-options)
args
(let [connection (or (:connection call-options)
global-connection)
uuid (.toString (java.util.UUID/randomUUID))]
(assert connection
(format (join "\n"
["No database connection supplied to function '%s',"
"Check the docs, and supply {:connection ...} as an option to the function call, or globally to the defquery declaration."])
name))
(cond (and (= insert-handler jdbc-fn) (:hooks query-options) (:before-insert @(:hooks query-options)))
((:before-insert @(:hooks query-options)) args statement (assoc call-options :uuid uuid))
(and (= execute-handler jdbc-fn) (:hooks query-options) (= "delete" (lower-case (first (split (trim statement) #" ")))) (:before-delete @(:hooks query-options)))
((:before-delete @(:hooks query-options)) args statement (assoc call-options :uuid uuid))
(and (= execute-handler jdbc-fn) (:hooks query-options) (= "update" (lower-case (first (split (trim statement) #" ")))) (:before-update @(:hooks query-options)))
((:before-update @(:hooks query-options)) args statement (assoc call-options :uuid uuid)))
(let [ret (safely (jdbc-fn connection
(rewrite-query-for-jdbc tokens args)
call-options)
:on-error
:max-retries 5
:retryable-error? #(isa? (class %) SQLException)
:retry-delay [:random-exp-backoff :base 50 :+/- 0.5])]
(cond (and (= insert-handler jdbc-fn) (:hooks query-options) (:after-insert @(:hooks query-options)))
((:after-insert @(:hooks query-options)) ret args statement (assoc call-options :uuid uuid))
(and (= execute-handler jdbc-fn) (:hooks query-options) (= "delete" (lower-case (first (split (trim statement) #" ")))) (:after-delete @(:hooks query-options)))
((:after-delete @(:hooks query-options)) ret args statement (assoc call-options :uuid uuid))
(and (= execute-handler jdbc-fn) (:hooks query-options) (= "update" (lower-case (first (split (trim statement) #" ")))) (:after-update @(:hooks query-options)))
((:after-update @(:hooks query-options)) ret args statement (assoc call-options :uuid uuid)))
ret))))
[display-args generated-function] (let [named-args (if-let [as-vec (seq (mapv (comp symbol clojure.core/name)
required-args))]
{:keys as-vec}
Expand Down
9 changes: 5 additions & 4 deletions test/yesql/generate_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
(:require [expectations :refer :all]
[clojure.template :refer [do-template]]
[yesql.statement-parser :refer [tokenize]]
[yesql.generate :refer :all]))
[yesql.generate :refer :all])
(:import [java.lang IllegalArgumentException]))

(do-template [statement _ expected-parameters]
(expect expected-parameters
Expand Down Expand Up @@ -82,13 +83,13 @@
"SELECT * FROM users WHERE group_ids IN(:group_ids) AND parent_id = :parent_id"
{:group_ids [1 2]
:parent_id 3}
=> ["SELECT * FROM users WHERE group_ids IN(?,?) AND parent_id = ?" 1 2 3])
=> ["SELECT * FROM users WHERE group_ids IN(?,?) AND parent_id = ?" 1 2 3])

;;; Incorrect parameters.
(expect AssertionError
(expect IllegalArgumentException
(rewrite-query-for-jdbc (tokenize "SELECT age FROM users WHERE country = :country AND name = :name")
{:country "gb"}))

(expect AssertionError
(expect IllegalArgumentException
(rewrite-query-for-jdbc (tokenize "SELECT age FROM users WHERE country = ? AND name = ?")
{}))