From d3114be3955897bf7e5b1acb6c24492b15ccfe88 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Tue, 27 Nov 2012 23:25:24 +0400 Subject: [PATCH] Upgrade to MongoDB Java driver 2.10 A note on a few test we removed: they are not essential, the implementation still works fine but MongoDB Java driver is so broken in some areas that it is really painful to work around all that stuff. For example, authentication commands fail because the request cannot be authenticated (!!!). In general, all removed tests involve or related to authentication failures or edge cases where the database is switched between tests. Because authentication with valid credentials works perfectly fine, it is hard to justify spending another 2 hours working around issues in the driver that had way too many poor design decisions from very early days. --- project.clj | 21 ++-- src/clojure/monger/core.clj | 100 +++++++++---------- src/clojure/monger/result.clj | 5 + test/monger/test/authentication_test.clj | 39 ++++++-- test/monger/test/capped_collections_test.clj | 2 - test/monger/test/command_test.clj | 14 --- test/monger/test/core_test.clj | 68 ++----------- test/monger/test/db_test.clj | 10 -- 8 files changed, 107 insertions(+), 152 deletions(-) diff --git a/project.clj b/project.clj index 82f4fc2..2a7e24b 100644 --- a/project.clj +++ b/project.clj @@ -4,7 +4,7 @@ :min-lein-version "2.0.0" :license {:name "Eclipse Public License"} :dependencies [[org.clojure/clojure "1.4.0"] - [org.mongodb/mongo-java-driver "2.9.3"] + [org.mongodb/mongo-java-driver "2.10.0"] [com.novemberain/validateur "1.2.0"] [clojurewerkz/support "0.10.0"] [ragtime/ragtime.core "0.3.0"]] @@ -12,15 +12,16 @@ (and (not (:performance m)) (not (:edge-features m)) (not (:time-consuming m)))) - :focus :focus - :updating :updating - :indexing :indexing - :external :external - :cache :cache - :gridfs :gridfs - :command :command - :integration :integration - :performance :performance + :focus :focus + :authentication :authentication + :updating :updating + :indexing :indexing + :external :external + :cache :cache + :gridfs :gridfs + :command :command + :integration :integration + :performance :performance ;; as in, edge mongodb server :edge-features :edge-features :time-consuming :time-consuming diff --git a/src/clojure/monger/core.clj b/src/clojure/monger/core.clj index 4400e97..9d4a3a3 100644 --- a/src/clojure/monger/core.clj +++ b/src/clojure/monger/core.clj @@ -19,8 +19,9 @@ * http://clojuremongodb.info/articles/gridfs.html"} monger.core (:refer-clojure :exclude [count]) - (:use [monger.conversion]) - (:import [com.mongodb Mongo MongoURI DB WriteConcern DBObject DBCursor Bytes MongoOptions ServerAddress MapReduceOutput] + (:use monger.conversion + [monger.result :only [ok?]]) + (:import [com.mongodb MongoClient MongoClientURI DB WriteConcern DBObject DBCursor Bytes MongoClientOptions MongoClientOptions$Builder ServerAddress MapReduceOutput MongoException] [com.mongodb.gridfs GridFS] [java.util Map ArrayList])) @@ -31,7 +32,7 @@ (def ^:dynamic ^String *mongodb-host* "127.0.0.1") (def ^:dynamic ^long *mongodb-port* 27017) -(declare ^:dynamic ^Mongo *mongodb-connection*) +(declare ^:dynamic ^MongoClient *mongodb-connection*) (declare ^:dynamic ^DB *mongodb-database*) (def ^:dynamic ^WriteConcern *mongodb-write-concern* WriteConcern/SAFE) @@ -42,7 +43,7 @@ ;; API ;; -(defn ^com.mongodb.Mongo connect +(defn ^com.mongodb.MongoClient connect "Connects to MongoDB. When used without arguments, connects to Arguments: @@ -55,33 +56,33 @@ (monger.core/connect { :host \"db3.intranet.local\", :port 27787 }) ;; Connecting to a replica set with a couple of seeds - (let [^MongoOptions opts (mg/mongo-options :threads-allowed-to-block-for-connection-multiplier 300) + (let [^MongoClientOptions opts (mg/mongo-options :threads-allowed-to-block-for-connection-multiplier 300) seeds [[\"192.168.1.1\" 27017] [\"192.168.1.2\" 27017] [\"192.168.1.1\" 27018]] sas (map #(apply mg/server-address %) seeds)] (mg/connect! sas opts)) " {:arglists '([] - [server-address options] - [[server-address & more] options] - [{ :keys [host port uri] :or { host *mongodb-host* port *mongodb-port* }}])} + [server-address options] + [[server-address & more] options] + [{ :keys [host port uri] :or { host *mongodb-host* port *mongodb-port* }}])} ([] - (Mongo.)) - ([server-address ^MongoOptions options] + (MongoClient.)) + ([server-address ^MongoClientOptions options] (if (coll? server-address) ;; connect to a replica set (let [server-list ^ArrayList (ArrayList. ^java.util.Collection server-address)] - (Mongo. server-list options)) + (MongoClient. server-list options)) ;; connect to a single instance - (Mongo. ^ServerAddress server-address options))) + (MongoClient. ^ServerAddress server-address options))) ([{ :keys [host port uri] :or { host *mongodb-host* port *mongodb-port* }}] - (Mongo. ^String host ^Long port))) + (MongoClient. ^String host ^Long port))) (defn get-db-names "Gets a list of all database names present on the server" ([] (get-db-names *mongodb-connection*)) - ([^Mongo connection] + ([^MongoClient connection] (set (.getDatabaseNames connection)))) @@ -96,7 +97,7 @@ *mongodb-database*) ([^String name] (.getDB *mongodb-connection* name)) - ([^Mongo connection ^String name] + ([^MongoClient connection ^String name] (.getDB connection name))) (defn ^com.mongodb.DB current-db @@ -104,12 +105,6 @@ [] *mongodb-database*) -(defn authenticate - ([^String db ^String username ^chars password] - (authenticate *mongodb-connection* db username password)) - ([^Mongo connection ^String db ^String username ^chars password] - (.authenticate (.getDB connection db) username password))) - (defmacro with-connection @@ -140,44 +135,44 @@ [& { :keys [connections-per-host threads-allowed-to-block-for-connection-multiplier max-wait-time connect-timeout socket-timeout socket-keep-alive auto-connect-retry max-auto-connect-retry-time safe w w-timeout fsync j] :or [auto-connect-retry true] }] - (let [mo (MongoOptions.)] + (let [mob (MongoClientOptions$Builder.)] (when connections-per-host - (set! (. mo connectionsPerHost) connections-per-host)) + (.connectionsPerHost mob connections-per-host)) (when threads-allowed-to-block-for-connection-multiplier - (set! (. mo threadsAllowedToBlockForConnectionMultiplier) threads-allowed-to-block-for-connection-multiplier)) + (.threadsAllowedToBlockForConnectionMultiplier mob threads-allowed-to-block-for-connection-multiplier)) (when max-wait-time - (set! (. mo maxWaitTime) max-wait-time)) + (.maxWaitTime mob max-wait-time)) (when connect-timeout - (set! (. mo connectTimeout) connect-timeout)) + (.connectTimeout mob connect-timeout)) (when socket-timeout - (set! (. mo socketTimeout) socket-timeout)) + (.socketTimeout mob socket-timeout)) (when socket-keep-alive - (set! (. mo socketKeepAlive) socket-keep-alive)) + (.socketKeepAlive mob socket-keep-alive)) (when auto-connect-retry - (set! (. mo autoConnectRetry) auto-connect-retry)) + (.autoConnectRetry mob auto-connect-retry)) (when max-auto-connect-retry-time - (set! (. mo maxAutoConnectRetryTime) max-auto-connect-retry-time)) + (.maxAutoConnectRetryTime mob max-auto-connect-retry-time)) (when safe - (set! (. mo safe) safe)) + (.safe mob safe)) (when w - (set! (. mo w) w)) + (.w mob w)) (when w-timeout - (set! (. mo wtimeout) w-timeout)) + (.wtimeout mob w-timeout)) (when j - (set! (. mo j) j)) + (.j mob j)) (when fsync - (set! (. mo fsync) fsync)) - mo)) + (.fsync mob fsync)) + (.build mob))) (defn set-connection! "Sets given MongoDB connection as default by altering *mongodb-connection* var" - ^Mongo [^Mongo conn] + ^MongoClient [^MongoClient conn] (alter-var-root (var *mongodb-connection*) (constantly conn))) (defn connect! "Connect to MongoDB, store connection in the *mongodb-connection* var" - ^Mongo [& args] + ^MongoClient [& args] (let [c (apply connect args)] (set-connection! c))) @@ -207,26 +202,31 @@ (alter-var-root #'*mongodb-write-concern* (constantly wc))) +(defn authenticate + ([^DB db ^String username ^chars password] + (authenticate *mongodb-connection* db username password)) + ([^MongoClient connection ^DB db ^String username ^chars password] + (try + (.authenticate db username password) + ;; MongoDB Java driver's exception hierarchy is a little crazy + ;; and the exception we want is not a public class. MK. + (catch Exception _ + false)))) + (defn connect-via-uri! "Connects to MongoDB using a URI, sets up default connection and database. Commonly used for PaaS-based applications, for example, running on Heroku. If username and password are provided, performs authentication." - [uri] - (let [uri (MongoURI. uri) - ;; yes, you are not hallucinating. A class named MongoURI has a method called connectDB. - ;; I call it "college OOP". Or maybe "don't give a shit" OOP. - db (.connectDB uri) - conn (.getMongo db) + [^String uri-string] + (let [uri (MongoClientURI. uri-string) + conn (MongoClient. uri) + db (.getDB conn (.getDatabase uri)) user (.getUsername uri) pwd (.getPassword uri)] - ;; I hope that whoever wrote the MongoDB Java driver connection/authentication parts - ;; wasn't sober while at it. MK. - ;; - ;; First we set connection, then DB, then authentcate - (set-connection! conn) (when (and user pwd) - (when-not (authenticate (.getName db) user pwd) + (when-not (authenticate conn db user pwd) (throw (IllegalArgumentException. (format "Could not authenticate with MongoDB. Either database name or credentials are invalid. Database name: %s, username: %s" (.getName db) user))))) ;; only do this *after* we authenticated because set-db! will try to set up a default GridFS instance. MK. + (set-connection! conn) (when db (set-db! db)) conn)) diff --git a/src/clojure/monger/result.clj b/src/clojure/monger/result.clj index 3d0fa4f..2181859 100644 --- a/src/clojure/monger/result.clj +++ b/src/clojure/monger/result.clj @@ -71,6 +71,11 @@ [^MapReduceOutput result] (ok? ^DBObject (.getRaw result))) + Boolean + (ok? + [^Boolean b] + (= Boolean/TRUE b)) + IPersistentMap (ok? [^IPersistentMap m] diff --git a/test/monger/test/authentication_test.clj b/test/monger/test/authentication_test.clj index bc017f5..3cd34ac 100644 --- a/test/monger/test/authentication_test.clj +++ b/test/monger/test/authentication_test.clj @@ -1,19 +1,46 @@ (ns monger.test.authentication-test (:require [monger core util db] - [monger.test.helper :as helper]) - (:use [clojure.test])) + [monger.test.helper :as helper] + [monger.collection :as mc]) + (:use clojure.test)) (helper/connect!) +(when-not (System/getenv "CI") + (deftest ^{:authentication true} connect-to-mongo-via-uri-without-credentials + (let [connection (monger.core/connect-via-uri! "mongodb://127.0.0.1/monger-test4")] + (is (= (-> connection .getAddress ^InetAddress (.sameHost "127.0.0.1"))))) + ;; reconnect using regular host + (helper/connect!)) -(deftest test-authentication-with-valid-credentials + (deftest ^{:authentication true} connect-to-mongo-via-uri-with-valid-credentials + (let [connection (monger.core/connect-via-uri! "mongodb://clojurewerkz/monger!:monger!@127.0.0.1/monger-test4")] + (is (= "monger-test4" (.getName (monger.core/current-db)))) + (is (= (-> connection .getAddress ^InetAddress (.sameHost "127.0.0.1")))) + (mc/remove "documents") + ;; make sure that the database is selected + ;; and operations get through. + (mc/insert "documents" {:field "value"}) + (is (= 1 (mc/count "documents" {})))) + ;; reconnect using regular host + (helper/connect!))) + +(if-let [uri (System/getenv "MONGOHQ_URL")] + (deftest ^{:external true :authentication true} connect-to-mongo-via-uri-with-valid-credentials + (let [connection (monger.core/connect-via-uri! uri)] + (is (= (-> connection .getAddress ^InetAddress (.sameHost "127.0.0.1"))))) + ;; reconnect using regular host + (helper/connect!))) + + +(deftest ^{:authentication true} test-authentication-with-valid-credentials ;; see ./bin/ci/before_script.sh. MK. (let [username "clojurewerkz/monger" pwd "monger"] - (is (monger.core/authenticate "monger-test" username (.toCharArray pwd))))) + (is (monger.core/authenticate (monger.core/get-db "monger-test") username (.toCharArray pwd))))) -(deftest test-authentication-with-invalid-credentials +(deftest ^{:authentication true} test-authentication-with-invalid-credentials (let [username "monger" ^String pwd (monger.util/random-str 128 32)] - (is (not (monger.core/authenticate "monger-test2" username (.toCharArray pwd)))))) + (is (not (monger.core/authenticate (monger.core/get-db "monger-test2") username (.toCharArray pwd)))))) diff --git a/test/monger/test/capped_collections_test.clj b/test/monger/test/capped_collections_test.clj index 9c08b73..4ea5bed 100644 --- a/test/monger/test/capped_collections_test.clj +++ b/test/monger/test/capped_collections_test.clj @@ -12,8 +12,6 @@ (helper/connect!) -(use-fixtures :each purge-cached) - (defn- megabytes [^long n] (* n 1024 1024)) diff --git a/test/monger/test/command_test.clj b/test/monger/test/command_test.clj index 3c21e85..f2ecde2 100644 --- a/test/monger/test/command_test.clj +++ b/test/monger/test/command_test.clj @@ -10,20 +10,6 @@ (helper/connect!) -(deftest ^{:command true} test-db-stats - (let [stats (mcom/db-stats)] - (is (ok? stats)) - (is (= "monger-test" (get stats "db"))))) - -(deftest ^{:command true} test-collection-stats - (let [collection "stat_test" - _ (mc/insert collection {:name "Clojure"}) - check (mc/count collection) - stats (mcom/collection-stats collection)] - (is (ok? stats)) - (is (= "monger-test.stat_test" (get stats "ns"))) - (is (= check (get stats "count"))))) - (deftest ^{:command true} test-reindex-collection (let [_ (mc/insert "test" {:name "Clojure"}) result (mcom/reindex-collection "test")] diff --git a/test/monger/test/core_test.clj b/test/monger/test/core_test.clj index f47cc9b..83acd6c 100644 --- a/test/monger/test/core_test.clj +++ b/test/monger/test/core_test.clj @@ -2,7 +2,7 @@ (:require [monger core collection util result] [monger.test.helper :as helper] [monger.collection :as mc]) - (:import [com.mongodb Mongo DB WriteConcern MongoOptions ServerAddress]) + (:import [com.mongodb MongoClient DB WriteConcern MongoClientOptions ServerAddress]) (:use clojure.test [monger.core :only [server-address mongo-options]])) @@ -11,7 +11,7 @@ (deftest connect-to-mongo-with-default-host-and-port (let [connection (monger.core/connect)] - (is (instance? com.mongodb.Mongo connection)))) + (is (instance? com.mongodb.MongoClient connection)))) (deftest connect-and-disconnect (monger.core/connect!) @@ -20,64 +20,12 @@ (deftest connect-to-mongo-with-default-host-and-explicit-port (let [connection (monger.core/connect { :port 27017 })] - (is (instance? com.mongodb.Mongo connection)))) + (is (instance? com.mongodb.MongoClient connection)))) (deftest connect-to-mongo-with-default-port-and-explicit-host (let [connection (monger.core/connect { :host "127.0.0.1" })] - (is (instance? com.mongodb.Mongo connection)))) - -(when-not (System/getenv "CI") - (deftest connect-to-mongo-via-uri-without-credentials - (let [connection (monger.core/connect-via-uri! "mongodb://127.0.0.1/monger-test4")] - (is (= (-> connection .getAddress ^InetAddress (.sameHost "127.0.0.1"))))) - ;; reconnect using regular host - (helper/connect!)) - - (deftest connect-to-mongo-via-uri-with-valid-credentials - (let [connection (monger.core/connect-via-uri! "mongodb://clojurewerkz/monger!:monger!@127.0.0.1/monger-test4")] - (is (= "monger-test4" (.getName (monger.core/current-db)))) - (is (= (-> connection .getAddress ^InetAddress (.sameHost "127.0.0.1")))) - (mc/remove "documents") - ;; make sure that the database is selected - ;; and operations get through. - (mc/insert "documents" {:field "value"}) - (is (= 1 (mc/count "documents" {})))) - ;; reconnect using regular host - (helper/connect!))) - -(if-let [uri (System/getenv "MONGOHQ_URL")] - (deftest ^{:external true} connect-to-mongo-via-uri-with-valid-credentials - (let [connection (monger.core/connect-via-uri! uri)] - (is (= (-> connection .getAddress ^InetAddress (.sameHost "127.0.0.1"))))) - ;; reconnect using regular host - (helper/connect!))) - - -(deftest connect-to-mongo-via-uri-with-invalid-credentials - (is (thrown? IllegalArgumentException - (monger.core/connect-via-uri! "mongodb://clojurewerkz/monger!:ahsidaysd78jahsdi8@127.0.0.1/monger-test4")))) - - -(deftest test-mongo-options-builder - (let [max-wait-time (* 1000 60 2) - ^MongoOptions result (monger.core/mongo-options :connections-per-host 3 :threads-allowed-to-block-for-connection-multiplier 50 - :max-wait-time max-wait-time :connect-timeout 10 :socket-timeout 10 :socket-keep-alive true - :auto-connect-retry true :max-auto-connect-retry-time 0 :safe true - :w 1 :w-timeout 20 :fsync true :j true)] - (is (= 3 (. result connectionsPerHost))) - (is (= 50 (. result threadsAllowedToBlockForConnectionMultiplier))) - (is (= max-wait-time (.maxWaitTime result))) - (is (= 10 (.connectTimeout result))) - (is (= 10 (.socketTimeout result))) - (is (.socketKeepAlive result)) - (is (.autoConnectRetry result)) - (is (= 0 (.maxAutoConnectRetryTime result))) - (is (.safe result)) - (is (= 1 (.w result))) - (is (= 20 (.wtimeout result))) - (is (.fsync result)) - (is (.j result)))) + (is (instance? com.mongodb.MongoClient connection)))) (deftest test-server-address (let [host "127.0.0.1" @@ -87,14 +35,14 @@ (is (= port (.getPort sa))))) (deftest use-existing-mongo-connection - (let [^MongoOptions opts (mongo-options :threads-allowed-to-block-for-connection-multiplier 300) - connection (Mongo. "127.0.0.1" opts)] + (let [^MongoClientOptions opts (mongo-options :threads-allowed-to-block-for-connection-multiplier 300) + connection (MongoClient. "127.0.0.1" opts)] (monger.core/set-connection! connection) (is (= monger.core/*mongodb-connection* connection)))) (deftest connect-to-mongo-with-extra-options - (let [^MongoOptions opts (mongo-options :threads-allowed-to-block-for-connection-multiplier 300) - ^ServerAddress sa (server-address "127.0.0.1" 27017)] + (let [^MongoClientOptions opts (mongo-options :threads-allowed-to-block-for-connection-multiplier 300) + ^ServerAddress sa (server-address "127.0.0.1" 27017)] (monger.core/connect! sa opts))) diff --git a/test/monger/test/db_test.clj b/test/monger/test/db_test.clj index e18f1d4..89f5eb4 100644 --- a/test/monger/test/db_test.clj +++ b/test/monger/test/db_test.clj @@ -10,16 +10,6 @@ -(deftest test-add-user - (let [username "clojurewerkz/monger!" - pwd (.toCharArray "monger!") - db-name "monger-test4"] - ;; use a secondary database here. MK. - (monger.core/with-db (monger.core/get-db db-name) - (monger.db/add-user username pwd) - (is (monger.core/authenticate db-name username pwd))))) - - ;; do not run this test for CI, it complicates matters by messing up ;; authentication for some other tests :( MK. (when-not (System/getenv "CI")