diff --git a/project.clj b/project.clj index 89521ec560e10ff753b8d5800be0c821903f3958..35a7c75aedc9c9bd57df96bb5abf99349d9920a6 100644 --- a/project.clj +++ b/project.clj @@ -27,8 +27,8 @@ [com.draines/postal "1.11.3"] ; SMTP library [com.h2database/h2 "1.4.187"] ; embedded SQL database [com.mattbertolini/liquibase-slf4j "1.2.1"] ; Java Migrations lib - [com.novemberain/monger "2.1.0"] ; MongoDB Driver - [compojure "1.3.4"] ; HTTP Routing library built on Ring + [com.novemberain/monger "3.0.0"] ; MongoDB Driver + [compojure "1.4.0"] ; HTTP Routing library built on Ring [environ "1.0.0"] ; easy environment management [hiccup "1.0.5"] ; HTML templating [korma "0.4.2"] ; SQL lib @@ -37,7 +37,7 @@ javax.jms/jms com.sun.jdmk/jmxtools com.sun.jmx/jmxri]] - [medley "0.6.0"] ; lightweight lib of useful functions + [medley "0.7.0"] ; lightweight lib of useful functions [org.liquibase/liquibase-core "3.4.0"] ; migration management (Java lib) [org.slf4j/slf4j-log4j12 "1.7.12"] [org.yaml/snakeyaml "1.15"] ; YAML parser (required by liquibase) diff --git a/resources/log4j.properties b/resources/log4j.properties index 9d4ab36d5642dd1ea6e8c4dde12e0fc571937fda..35f3fff6ee338862061fffd79965bed531d2cf35 100644 --- a/resources/log4j.properties +++ b/resources/log4j.properties @@ -18,3 +18,4 @@ log4j.appender.file.layout.ConversionPattern=%d [%t] %-5p%c - %m%n log4j.logger.com.mchange=WARN log4j.logger.liquibase=WARN log4j.logger.metabase=DEBUG +log4j.logger.org.mongodb=WARN diff --git a/src/metabase/driver/mongo.clj b/src/metabase/driver/mongo.clj index e43ae04af61d437ca8ce39d86039732bf07d12f1..60e4e3bd0a84725cf2166cf19060b0d7bcf91f8c 100644 --- a/src/metabase/driver/mongo.clj +++ b/src/metabase/driver/mongo.clj @@ -24,7 +24,7 @@ (defn- table->column-names "Return a set of the column names for TABLE." [table] - (with-mongo-connection [^com.mongodb.DBApiLayer conn @(:db table)] + (with-mongo-connection [^com.mongodb.DB conn @(:db table)] (->> (mc/find-maps conn (:name table)) (take max-sync-lazy-seq-results) (map keys) @@ -46,7 +46,7 @@ IDriver ;;; ### Connection (can-connect? [_ database] - (with-mongo-connection [^com.mongodb.DBApiLayer conn database] + (with-mongo-connection [^com.mongodb.DB conn database] (= (-> (cmd/db-stats conn) (conv/from-db-object :keywordize) :ok) @@ -58,7 +58,7 @@ ;;; ### QP (wrap-process-query-middleware [_ qp] (fn [query] - (with-mongo-connection [^com.mongodb.DBApiLayer conn (:database query)] + (with-mongo-connection [^com.mongodb.DB conn (:database query)] (qp query)))) (process-query [_ query] @@ -70,7 +70,7 @@ (do-sync-fn))) (active-table-names [_ database] - (with-mongo-connection [^com.mongodb.DBApiLayer conn database] + (with-mongo-connection [^com.mongodb.DB conn database] (-> (mdb/get-collection-names conn) (set/difference #{"system.indexes"})))) diff --git a/src/metabase/driver/mongo/query_processor.clj b/src/metabase/driver/mongo/query_processor.clj index 3ae8065517f76c75abfacfefa94580f1da189b40..972c0106e31df7b6225126de55e9741c61676689 100644 --- a/src/metabase/driver/mongo/query_processor.clj +++ b/src/metabase/driver/mongo/query_processor.clj @@ -20,7 +20,7 @@ [metabase.models.field :refer [Field]] [metabase.util :as u]) (:import (com.mongodb CommandResult - DBApiLayer) + DB) (clojure.lang PersistentArrayMap) (org.bson.types ObjectId))) @@ -61,7 +61,7 @@ -> {\"_id\" \"01001\", \"city\" \"AGAWAM\", ...}" [^String command] (assert *mongo-connection* "eval-raw-command must be ran inside the body of with-mongo-connection.") - (let [^CommandResult result (.doEval ^DBApiLayer *mongo-connection* command nil)] + (let [^CommandResult result (.doEval ^DB *mongo-connection* command nil)] (when-not (.ok result) (throw (.getException result))) (let [{result "retval"} (PersistentArrayMap/create (.toMap result))] @@ -82,7 +82,7 @@ (defn aggregate "Generate a Monger `aggregate` form." [& forms] - `(mc/aggregate ^DBApiLayer *mongo-connection* ~*collection-name* [~@(when *constraints* + `(mc/aggregate ^DB *mongo-connection* ~*collection-name* [~@(when *constraints* [{$match *constraints*}]) ~@(filter identity forms)])) @@ -99,16 +99,16 @@ (format "$%s" (field->name field))) (defn- aggregation:rows [] - `(doall (with-collection ^DBApiLayer *mongo-connection* ~*collection-name* + `(doall (with-collection ^DB *mongo-connection* ~*collection-name* ~@(when *constraints* [`(find ~*constraints*)]) ~@(mapcat apply-clause (dissoc (:query *query*) :filter))))) (defn- aggregation:count ([] - `[{:count (mc/count ^DBApiLayer *mongo-connection* ~*collection-name* + `[{:count (mc/count ^DB *mongo-connection* ~*collection-name* ~*constraints*)}]) ([field] - `[{:count (mc/count ^DBApiLayer *mongo-connection* ~*collection-name* + `[{:count (mc/count ^DB *mongo-connection* ~*collection-name* (merge ~*constraints* {~(field->name field) {$exists true}}))}])) @@ -262,7 +262,7 @@ "Used by `defclause` to store the clause definitions generated by it." (atom '())) -(defmacro defclause +(defmacro ^:private defclause "Generate a new clause definition that will be called inside of a `match` statement whenever CLAUSE matches MATCH-BINDING. @@ -272,7 +272,7 @@ `(swap! clauses concat '[[~clause ~match-binding] (try ~@body (catch Throwable e# - (log/error (color/red ~(format "Failed to process clause [%s %s]: " clause match-binding) + (log/error (color/red ~(format "Failed to process '%s' clause:" (name clause)) (.getMessage e#)))))])) ;; ### CLAUSE DEFINITIONS @@ -287,9 +287,11 @@ (defn- format-value "Convert ID strings to `ObjectId`." [{:keys [field-name base-type value]}] - (if (and (= field-name "_id") - (= base-type :UnknownField)) `(ObjectId. ~value) - value)) + (cond + (and (= field-name "_id") + (= base-type :UnknownField)) `(ObjectId. ~value) + (= (type value) java.sql.Timestamp) (java.util.Date. (.getTime ^java.sql.Timestamp value)) ; ugg + :else value)) (defn- parse-filter-subclause [{:keys [filter-type field value] :as filter}] (let [field (when field (field->name field)) diff --git a/src/metabase/driver/mongo/util.clj b/src/metabase/driver/mongo/util.clj index a5af321ec476a53d029090b9f36535541a4139a1..75003e2b3660595db2bc95ce45115e8ca09bdea7 100644 --- a/src/metabase/driver/mongo/util.clj +++ b/src/metabase/driver/mongo/util.clj @@ -3,7 +3,8 @@ (:require [clojure.string :as s] [clojure.tools.logging :as log] [colorize.core :as color] - [monger.core :as mg] + (monger [core :as mg] + [credentials :as mcred]) [metabase.driver :as driver])) (def ^:const ^:private connection-timeout-ms @@ -16,39 +17,34 @@ on *one* occasion." 1500) -(defn- details-map->connection-string - [{:keys [user pass host port dbname]}] - {:pre [host - dbname]} - (str "mongodb://" - user - (when-not (s/blank? pass) - (assert (not (s/blank? user)) "Can't have a password without a user!") - (str ":" pass)) - (when-not (s/blank? user) "@") - host - (when-not (s/blank? (str port)) - (str ":" port)) - "/" - dbname - "?connectTimeoutMS=" - connection-timeout-ms)) - -(def ^:dynamic ^com.mongodb.DBApiLayer *mongo-connection* +(def ^:dynamic ^com.mongodb.DB *mongo-connection* "Connection to a Mongo database. Bound by top-level `with-mongo-connection` so it may be reused within its body." nil) +(def ^:private mongo-connection-options + ;; Have to use the Java builder directly since monger's wrapper method doesn't support .serverSelectionTimeout :unamused: + (let [opts (com.mongodb.MongoClientOptions$Builder.)] + (-> opts + (.connectTimeout connection-timeout-ms) + (.serverSelectionTimeout connection-timeout-ms) + (.build)))) + (defn -with-mongo-connection "Run F with a new connection (bound to `*mongo-connection*`) to DATABASE. Don't use this directly; use `with-mongo-connection`." [f database] - (let [connection-string (cond - (string? database) database - (:dbname (:details database)) (details-map->connection-string (:details database)) ; new-style -- entire Database obj - (:dbname database) (details-map->connection-string database) ; new-style -- connection details map only - :else (throw (Exception. (str "with-mongo-connection failed: bad connection details:" (:details database))))) - {conn :conn, mongo-connection :db} (mg/connect-via-uri connection-string)] + (let [{:keys [dbname host port user pass] + :or {port 27017, pass ""}} (cond + (string? database) {:dbname database} + (:dbname (:details database)) (:details database) ; entire Database obj + (:dbname database) database ; connection details map only + :else (throw (Exception. (str "with-mongo-connection failed: bad connection details:" (:details database))))) + server-address (mg/server-address host port) + credentials (when user + (mcred/create user dbname pass)) + conn (mg/connect server-address mongo-connection-options credentials) + mongo-connection (mg/get-db conn dbname)] (log/debug (color/cyan "<< OPENED NEW MONGODB CONNECTION >>")) (try (binding [*mongo-connection* mongo-connection] @@ -62,11 +58,11 @@ (We're smart about it: DATABASE isn't even evaluated if `*mongo-connection*` is already bound.) ;; delay isn't derefed if *mongo-connection* is already bound - (with-mongo-connection [^com.mongodb.DBApiLayer conn @(:db (sel :one Table ...))] + (with-mongo-connection [^com.mongodb.DB conn @(:db (sel :one Table ...))] ...) ;; You can use a string instead of a Database - (with-mongo-connection [^com.mongodb.DBApiLayer conn \"mongodb://127.0.0.1:27017/test\"] + (with-mongo-connection [^com.mongodb.DB conn \"mongodb://127.0.0.1:27017/test\"] ...) DATABASE-OR-CONNECTION-STRING can also optionally be the connection details map on its own." diff --git a/src/metabase/util.clj b/src/metabase/util.clj index d5296bae8bfac8f5de547f135d077c76c4890924..2b6de3451fbf6b464a2eca17e82719bb297a7bf1 100644 --- a/src/metabase/util.clj +++ b/src/metabase/util.clj @@ -109,10 +109,7 @@ (do (.close reader) acc)))) -(defn - ^{:arglists ([pred? args] - [pred? args default])} - optional +(defn optional "Helper function for defining functions that accept optional arguments. If PRED? is true of the first item in ARGS, a pair like `[first-arg other-args]` is returned; otherwise, a pair like `[DEFAULT other-args]` is returned. @@ -126,6 +123,8 @@ {k nums})) (wrap-nums 1 2 3) -> {:nums [1 2 3]} (wrap-nums :numbers 1 2 3) -> {:numbers [1 2 3]}" + {:arglists '([pred? args] + [pred? args default])} [pred? args & [default]] (if (pred? (first args)) [(first args) (next args)] [default args])) diff --git a/test/metabase/api/common/internal_test.clj b/test/metabase/api/common/internal_test.clj index 841f2a3a14c461b7d780df82c7ac0acfe9e5607f..a3d65224c2e0ecb28f013dcb0572e64fe4b20f9b 100644 --- a/test/metabase/api/common/internal_test.clj +++ b/test/metabase/api/common/internal_test.clj @@ -1,6 +1,6 @@ (ns metabase.api.common.internal-test (:require [expectations :refer :all] - [medley.core :as medley] + [medley.core :as m] (metabase.api.common [internal :refer :all]))) ;;; TESTS FOR ROUTE-FN-NAME @@ -32,8 +32,8 @@ ;; expectations (internally, `clojure.data/diff`) doesn't think two regexes with the same exact pattern are equal. ;; so in order to make sure we're getting back the right output we'll just change them to strings, e.g. `#"[0-9]+" -> "#[0-9]+"` (defmacro no-regex [& body] - `(binding [*auto-parse-types* (medley/map-vals #(medley/update % :route-param-regex (partial str "#")) - *auto-parse-types*) ] + `(binding [*auto-parse-types* (m/map-vals #(update % :route-param-regex (partial str "#")) + *auto-parse-types*) ] ~@body)) (expect nil diff --git a/test/metabase/api/session_test.clj b/test/metabase/api/session_test.clj index c117397e6468f52a79a111c5c552e5baef955762..21d46f4670f4bb29a20c581fc74ab03149ec88a1 100644 --- a/test/metabase/api/session_test.clj +++ b/test/metabase/api/session_test.clj @@ -106,7 +106,6 @@ (let [{:keys [email id]} (create-user :password "password", :last_name user-last-name, :reset_triggered (System/currentTimeMillis)) token (str id "_" (java.util.UUID/randomUUID)) _ (upd User id :reset_token token)] - (println token) ;; run the password reset (metabase.http-client/client :post 200 "session/reset_password" {:token token :password "whateverUP12!!"})))) diff --git a/test/metabase/test/data/mongo.clj b/test/metabase/test/data/mongo.clj index 9a902810e3b744431122793d759fe0edf9ff2823..8e21f47110cffbdcd53d3d0783ac1d923624557a 100644 --- a/test/metabase/test/data/mongo.clj +++ b/test/metabase/test/data/mongo.clj @@ -29,22 +29,28 @@ (create-physical-table! [_ _ _]) (drop-physical-table! [this database-definition {:keys [table-name]}] - (with-mongo-connection [^com.mongodb.DBApiLayer mongo-db (database->connection-details this database-definition)] + (with-mongo-connection [^com.mongodb.DB mongo-db (database->connection-details this database-definition)] (mc/drop mongo-db (name table-name)))) (load-table-data! [this database-definition {:keys [field-definitions table-name rows]}] - (with-mongo-connection [^com.mongodb.DBApiLayer mongo-db (database->connection-details this database-definition)] + (with-mongo-connection [^com.mongodb.DB mongo-db (database->connection-details this database-definition)] (let [field-names (->> field-definitions (map :field-name) (map keyword))] ;; Use map-indexed so we can get an ID for each row (index + 1) (doseq [[i row] (map-indexed (partial vector) rows)] - (try - ;; Insert each row - (mc/insert mongo-db (name table-name) (assoc (zipmap field-names row) - :_id (inc i))) - ;; If row already exists then nothing to do - (catch com.mongodb.MongoException$DuplicateKey _))))))) + (let [row (for [v row] + ;; Conver all the java.sql.Timestamps to java.util.Date, because the Mongo driver insists on being obnoxious and going from + ;; using Timestamps in 2.x to Dates in 3.x + (if (= (type v) java.sql.Timestamp) + (java.util.Date. (.getTime ^java.sql.Timestamp v)) + v))] + (try + ;; Insert each row + (mc/insert mongo-db (name table-name) (assoc (zipmap field-names row) + :_id (inc i))) + ;; If row already exists then nothing to do + (catch com.mongodb.MongoException _)))))))) (defn ^MongoDatasetLoader dataset-loader [] (->MongoDatasetLoader)) diff --git a/test_resources/log4j.properties b/test_resources/log4j.properties index c2e7b0ff2c176e956b635b668a1d84c07dc6ee6e..e83f705f34d344621fb6a7f0f3322a2c6765928a 100644 --- a/test_resources/log4j.properties +++ b/test_resources/log4j.properties @@ -18,3 +18,4 @@ log4j.appender.file.layout.ConversionPattern=%d [%t] %-5p%c - %m%n log4j.logger.com.mchange=WARN log4j.logger.liquibase=WARN log4j.logger.metabase=INFO +log4j.logger.org.mongodb=WARN