From aa792fbee2cc0dd8306bab5338e0c73609b5dd14 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 28 Jan 2019 09:32:01 -0700 Subject: [PATCH 1/2] Explicitly share a single connection in tests Right now the way our test suite works is that we set the database connection pool size to 1, and tell that single connection to start a test transaction. This works, but has the caveat that only a single reference to the connection can be live at any given time. This becomes a problem when we add background jobs into the mix, which will always want to grab two connections from the pool (and we still need them to be the same connection). The code that we end up running looks something like: ``` // In request let conn = pool.get(); insert_background_job(&conn)?; // In middleware that runs jobs for _ in 0..job_count { thread::spawn(|| { conn = pool.get(); lock_job(&conn); // in actual job conn = pool.get(); do_stuff(&conn); unlock_or_delete_job(&conn); }); } ``` Note that the worker and the job itself both need a connection. In order to make this work, we need to change our pool to actually just be a single connection given out multiple times in test mode. I've chosen to use a reentrant mutex for this for two reasons: 1. We need to satisfy the `Sync` bound on `App`, even though we're only running in a single thread in tests. 2. The background worker actually does end up running in another thread, so we do actually need to be thread safe. The result of this is that we can have the connection checked out more than once at the same time, but we still can't be more relaxed about when it gets dropped in our test code, since we need it to be unlocked if we try to get a connection from another thread as the result of running a background job. Our tests shouldn't know or care about whether background jobs were run async or not, so they should assume every request might run one. The mutex guard holds a reference to the mutex, so this introduces a lifetime constraint that hasn't existed since the Diesel port. The return type of `req.db_conn()` is now effectively `&PgConnection` instead of `Box`. Since the method exists on `Request`, this means it gets treated as an immutable borrow of the whole request. The fixes are all pretty simple, and boil down to either cloning the app and getting the DB directly from there, taking immutable references instead of mutable ones, or dropping the connection when we're done with it. --- Cargo.lock | 188 +++++++++++++++++++++- Cargo.toml | 1 + src/app.rs | 3 +- src/controllers/crate_owner_invitation.rs | 8 +- src/controllers/krate/publish.rs | 2 +- src/controllers/krate/search.rs | 2 +- src/db.rs | 52 +++++- src/middleware/current_user.rs | 4 + src/tests/all.rs | 13 ++ src/tests/util.rs | 5 +- 10 files changed, 261 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e380034b6c..e3285ea47e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -60,6 +60,11 @@ dependencies = [ "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "autocfg" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "backtrace" version = "0.3.3" @@ -183,6 +188,7 @@ dependencies = [ "oauth2 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "old_semver 0.1.0", "openssl 0.10.13 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.18 (registry+https://github.com/rust-lang/crates.io-index)", "reqwest 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)", "scheduled-thread-pool 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1125,6 +1131,15 @@ name = "license-exprs" version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "lock_api" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "owning_ref 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "scopeguard 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "log" version = "0.3.8" @@ -1405,6 +1420,35 @@ dependencies = [ "vcpkg 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "owning_ref" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "stable_deref_trait 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "parking_lot" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "lock_api 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot_core 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "parking_lot_core" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)", + "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", + "rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", + "smallvec 0.6.8 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "percent-encoding" version = "1.0.0" @@ -1557,14 +1601,121 @@ dependencies = [ "cloudabi 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)", "fuchsia-zircon 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)", - "rand_core 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_core 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rand" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "autocfg 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_chacha 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_core 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_hc 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_isaac 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_jitter 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_os 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_pcg 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_xorshift 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "rand_chacha" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "autocfg 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_core 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "rand_core" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rand_core 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rand_core" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rand_core 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rand_core" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + +[[package]] +name = "rand_hc" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rand_core 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rand_isaac" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rand_core 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rand_jitter" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_core 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rand_os" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "cloudabi 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "fuchsia-zircon 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_core 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", + "rdrand 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rand_pcg" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rand_core 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", + "rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rand_xorshift" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rand_core 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rdrand" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rand_core 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", +] [[package]] name = "redox_syscall" @@ -1845,6 +1996,14 @@ name = "slab" version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "smallvec" +version = "0.6.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "unreachable 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "socket2" version = "0.2.4" @@ -1857,6 +2016,11 @@ dependencies = [ "ws2_32-sys 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "stable_deref_trait" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "string" version = "0.1.0" @@ -2461,6 +2625,7 @@ dependencies = [ "checksum arrayvec 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)" = "a1e964f9e24d588183fcb43503abda40d288c8657dfc27311516ce2f05675aef" "checksum ascii_utils 0.9.3 (registry+https://github.com/rust-lang/crates.io-index)" = "71938f30533e4d95a6d17aa530939da3842c2ab6f4f84b9dae68447e4129f74a" "checksum atty 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)" = "2fc4a1aa4c24c0718a250f0681885c1af91419d242f29eb8f2ab28502d80dbd1" +"checksum autocfg 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "a6d640bee2da49f60a4068a7fae53acde8982514ab7bae8b8cea9e88cbcfd799" "checksum backtrace 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "99f2ce94e22b8e664d95c57fff45b98a966c2252b60691d0b7aeeccd88d70983" "checksum backtrace-sys 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)" = "44585761d6161b0f57afc49482ab6bd067e4edef48c12a152c237eb0203f7661" "checksum base64 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "96434f987501f0ed4eb336a411e0631ecd1afa11574fe148587adc4ff96143c9" @@ -2573,6 +2738,7 @@ dependencies = [ "checksum libssh2-sys 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)" = "0db4ec23611747ef772db1c4d650f8bd762f07b461727ec998f953c614024b75" "checksum libz-sys 1.0.18 (registry+https://github.com/rust-lang/crates.io-index)" = "87f737ad6cc6fd6eefe3d9dc5412f1573865bded441300904d2f42269e140f16" "checksum license-exprs 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e66da8cfc5d7882ef44dfae506db874873bb7596bc89468c30afbdb47e115593" +"checksum lock_api 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "62ebf1391f6acad60e5c8b43706dde4582df75c06698ab44511d15016bc2442c" "checksum log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)" = "880f77541efa6e5cc74e76910c9884d9859683118839d6a1dc3b11e63512565b" "checksum log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)" = "d4fcce5fa49cc693c312001daf1d13411c4a5283796bac1084299ea3e567113f" "checksum mac 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" @@ -2604,6 +2770,9 @@ dependencies = [ "checksum openssl 0.10.13 (registry+https://github.com/rust-lang/crates.io-index)" = "5af9e83eb3c51ee806387d26a43056f3246d865844caa6dd704d2ba7e831c264" "checksum openssl-probe 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "d98df0270d404ccd3c050a41d579c52d1db15375168bb3471e04ec0f5f378daf" "checksum openssl-sys 0.9.38 (registry+https://github.com/rust-lang/crates.io-index)" = "ff3d1b390ab1b9700f682ad95a30dc9c0f40dd212ca57266012cfc678b0e365a" +"checksum owning_ref 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "49a4b8ea2179e6a2e27411d3bca09ca6dd630821cf6894c6c7c8467a8ee7ef13" +"checksum parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ab41b4aed082705d1056416ae4468b6ea99d52599ecf3169b00088d43113e337" +"checksum parking_lot_core 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "94c8c7923936b28d546dfd14d4472eaf34c99b14e1c973a32b3e6d4eb04298c9" "checksum percent-encoding 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "de154f638187706bde41d9b4738748933d64e6b37bdbffc0b47a97d16a6ae356" "checksum pest 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)" = "0fce5d8b5cc33983fc74f78ad552b5522ab41442c4ca91606e4236eb4b5ceefc" "checksum pest_derive 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)" = "ab94faafeb93f4c5e3ce81ca0e5a779529a602ad5d09ae6d21996bfb8b6a52bf" @@ -2624,7 +2793,18 @@ dependencies = [ "checksum rand 0.3.18 (registry+https://github.com/rust-lang/crates.io-index)" = "6475140dfd8655aeb72e1fd4b7a1cc1c202be65d71669476e392fe62532b9edd" "checksum rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "eba5f8cb59cc50ed56be8880a5c7b496bfd9bd26394e176bc67884094145c2c5" "checksum rand 0.5.5 (registry+https://github.com/rust-lang/crates.io-index)" = "e464cd887e869cddcae8792a4ee31d23c7edd516700695608f5b98c67ee0131c" -"checksum rand_core 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "edecf0f94da5551fc9b492093e30b041a891657db7940ee221f9d2f66e82eef2" +"checksum rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)" = "6d71dacdc3c88c1fde3885a3be3fbab9f35724e6ce99467f7d9c5026132184ca" +"checksum rand_chacha 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "556d3a1ca6600bfcbab7c7c91ccb085ac7fbbcd70e008a98742e7847f4f7bcef" +"checksum rand_core 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "1961a422c4d189dfb50ffa9320bf1f2a9bd54ecb92792fb9477f99a1045f3372" +"checksum rand_core 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7a6fdeb83b075e8266dcc8762c22776f6877a63111121f5f8c7411e5be7eed4b" +"checksum rand_core 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d0e7a549d590831370895ab7ba4ea0c1b6b011d106b5ff2da6eee112615e6dc0" +"checksum rand_hc 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7b40677c7be09ae76218dc623efbf7b18e34bced3f38883af07bb75630a21bc4" +"checksum rand_isaac 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ded997c9d5f13925be2a6fd7e66bf1872597f759fd9dd93513dd7e92e5a5ee08" +"checksum rand_jitter 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f47842851e13bd803b506bdd1345328e0a1394733ee58e627b5e39332b9afafe" +"checksum rand_os 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "f46fbd5550acf75b0c2730f5dd1873751daf9beb8f11b44027778fae50d7feca" +"checksum rand_pcg 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "086bd09a33c7044e56bb44d5bdde5a60e7f119a9e95b0775f545de759a32fe05" +"checksum rand_xorshift 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cbf7e9e623549b0e21f6e97cf8ecf247c1a8fd2e8a992ae265314300b2455d5c" +"checksum rdrand 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "678054eb77286b51581ba43620cc911abf02758c91f93f479767aed0f90458b2" "checksum redox_syscall 0.1.31 (registry+https://github.com/rust-lang/crates.io-index)" = "8dde11f18c108289bef24469638a04dce49da56084f2d50618b226e47eb04509" "checksum redox_termios 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7e891cfe48e9100a70a3b6eb652fef28920c117d366339687bd5576160db0f76" "checksum regex 0.1.80 (registry+https://github.com/rust-lang/crates.io-index)" = "4fd4ace6a8cf7860714a2c2280d6c1f7e6a413486c13298bbc86fd3da019402f" @@ -2659,7 +2839,9 @@ dependencies = [ "checksum serde_urlencoded 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)" = "aaed41d9fb1e2f587201b863356590c90c1157495d811430a0c0325fe8169650" "checksum siphasher 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "0df90a788073e8d0235a67e50441d47db7c8ad9debd91cbf43736a2a92d36537" "checksum slab 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "5f9776d6b986f77b35c6cf846c11ad986ff128fe0b2b63a3628e3755e8d3102d" +"checksum smallvec 0.6.8 (registry+https://github.com/rust-lang/crates.io-index)" = "88aea073965ab29f6edb5493faf96ad662fb18aa9eeb186a3b7057951605ed15" "checksum socket2 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)" = "36b4896961171cd3317c7e9603d88f379f8c6e45342212235d356496680c68fd" +"checksum stable_deref_trait 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "dba1a27d3efae4351c8051072d619e3ade2820635c3958d826bfea39d59b54c8" "checksum string 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "31f98b200e7caca9efca50fc0aa69cd58a5ec81d5f6e75b2f3ecaad2e998972a" "checksum string_cache 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "413fc7852aeeb5472f1986ef755f561ddf0c789d3d796e65f0b6fe293ecd4ef8" "checksum string_cache_codegen 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "479cde50c3539481f33906a387f2bd17c8e87cb848c35b6021d41fb81ff9b4d7" diff --git a/Cargo.toml b/Cargo.toml index 8b315edbc9f..fb9c6c87faf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,6 +62,7 @@ scheduled-thread-pool = "0.2.0" derive_deref = "1.0.0" reqwest = "0.9.1" tempdir = "0.3.7" +parking_lot = "0.7.1" lettre = {git = "https://github.com/lettre/lettre", version = "0.9"} lettre_email = {git = "https://github.com/lettre/lettre", version = "0.9"} diff --git a/src/app.rs b/src/app.rs index bdc42eea0f5..41591ae9f5e 100644 --- a/src/app.rs +++ b/src/app.rs @@ -73,6 +73,7 @@ impl App { let db_connection_timeout = match (env::var("DB_TIMEOUT"), config.env) { (Ok(num), _) => num.parse().expect("couldn't parse DB_TIMEOUT"), (_, Env::Production) => 10, + (_, Env::Test) => 1, _ => 30, }; @@ -88,7 +89,7 @@ impl App { let repo = git2::Repository::open(&config.git_repo_checkout).unwrap(); App { - diesel_database: db::diesel_pool(&config.db_url, diesel_db_config), + diesel_database: db::diesel_pool(&config.db_url, config.env, diesel_db_config), github, session_key: config.session_key.clone(), git_repo: Mutex::new(repo), diff --git a/src/controllers/crate_owner_invitation.rs b/src/controllers/crate_owner_invitation.rs index df5a9b2b1dd..89cf5c35cdc 100644 --- a/src/controllers/crate_owner_invitation.rs +++ b/src/controllers/crate_owner_invitation.rs @@ -32,11 +32,11 @@ struct OwnerInvitation { /// Handles the `PUT /me/crate_owner_invitations/:crate_id` route. pub fn handle_invite(req: &mut dyn Request) -> CargoResult { - let conn = &*req.db_conn()?; - let mut body = String::new(); req.body().read_to_string(&mut body)?; + let conn = &*req.db_conn()?; + let crate_invite: OwnerInvitation = serde_json::from_str(&body).map_err(|_| human("invalid json request"))?; @@ -50,7 +50,7 @@ pub fn handle_invite(req: &mut dyn Request) -> CargoResult { } fn accept_invite( - req: &mut dyn Request, + req: &dyn Request, conn: &PgConnection, crate_invite: InvitationResponse, ) -> CargoResult { @@ -88,7 +88,7 @@ fn accept_invite( } fn decline_invite( - req: &mut dyn Request, + req: &dyn Request, conn: &PgConnection, crate_invite: InvitationResponse, ) -> CargoResult { diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index c7f94f85554..b7e8931a098 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -63,7 +63,7 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { let categories = new_crate.categories.as_ref().map(|s| &s[..]).unwrap_or(&[]); let categories: Vec<_> = categories.iter().map(|k| &***k).collect(); - let conn = req.db_conn()?; + let conn = app.diesel_database.get()?; let mut other_warnings = vec![]; if !user.has_verified_email(&conn)? { diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs index 5dd9b5419ee..0e0aebee667 100644 --- a/src/controllers/krate/search.rs +++ b/src/controllers/krate/search.rs @@ -171,7 +171,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult { let badges = CrateBadge::belonging_to(&crates) .select((badges::crate_id, badges::all_columns)) - .load::(&conn)? + .load::(&*conn)? .grouped_by(&crates) .into_iter() .map(|badges| badges.into_iter().map(|cb| cb.badge).collect()); diff --git a/src/db.rs b/src/db.rs index cc966d3f893..33503bea7e0 100644 --- a/src/db.rs +++ b/src/db.rs @@ -3,13 +3,49 @@ use std::env; use conduit::Request; use diesel::prelude::*; use diesel::r2d2::{self, ConnectionManager, CustomizeConnection}; +use parking_lot::{ReentrantMutex, ReentrantMutexGuard}; +use std::ops::Deref; use url::Url; use crate::middleware::app::RequestApp; use crate::util::CargoResult; +use crate::Env; -pub type DieselPool = r2d2::Pool>; -type DieselPooledConn = r2d2::PooledConnection>; +#[allow(missing_debug_implementations)] +pub enum DieselPool { + Pool(r2d2::Pool>), + Test(ReentrantMutex), +} + +impl DieselPool { + pub fn get(&self) -> CargoResult { + match self { + DieselPool::Pool(pool) => Ok(DieselPooledConn::Pool(pool.get()?)), + DieselPool::Test(conn) => Ok(DieselPooledConn::Test(conn.lock())), + } + } + + fn test_conn(conn: PgConnection) -> Self { + DieselPool::Test(ReentrantMutex::new(conn)) + } +} + +#[allow(missing_debug_implementations)] +pub enum DieselPooledConn<'a> { + Pool(r2d2::PooledConnection>), + Test(ReentrantMutexGuard<'a, PgConnection>), +} + +impl Deref for DieselPooledConn<'_> { + type Target = PgConnection; + + fn deref(&self) -> &Self::Target { + match self { + DieselPooledConn::Pool(conn) => conn.deref(), + DieselPooledConn::Test(conn) => conn.deref(), + } + } +} pub fn connect_now() -> ConnectionResult { use diesel::Connection; @@ -22,14 +58,22 @@ pub fn connect_now() -> ConnectionResult { pub fn diesel_pool( url: &str, + env: Env, config: r2d2::Builder>, ) -> DieselPool { let mut url = Url::parse(url).expect("Invalid database URL"); if env::var("HEROKU").is_ok() && !url.query_pairs().any(|(k, _)| k == "sslmode") { url.query_pairs_mut().append_pair("sslmode", "require"); } - let manager = ConnectionManager::new(url.into_string()); - config.build(manager).unwrap() + + if env == Env::Test { + let conn = + PgConnection::establish(&url.into_string()).expect("failed to establish connection"); + DieselPool::test_conn(conn) + } else { + let manager = ConnectionManager::new(url.into_string()); + DieselPool::Pool(config.build(manager).unwrap()) + } } pub trait RequestTransaction { diff --git a/src/middleware/current_user.rs b/src/middleware/current_user.rs index 6ff9c05e64a..577d1ac2040 100644 --- a/src/middleware/current_user.rs +++ b/src/middleware/current_user.rs @@ -20,6 +20,8 @@ pub enum AuthenticationSource { impl Middleware for CurrentUser { fn before(&self, req: &mut dyn Request) -> Result<(), Box> { + use std::mem::drop; + // Check if the request has a session cookie with a `user_id` property inside let id = { req.session() @@ -32,6 +34,7 @@ impl Middleware for CurrentUser { if let Some(id) = id { // If it did, look for a user in the database with the given `user_id` let maybe_user = users::table.find(id).first::(&*conn); + drop(conn); if let Ok(user) = maybe_user { // Attach the `User` model from the database to the request req.mut_extensions().insert(user); @@ -46,6 +49,7 @@ impl Middleware for CurrentUser { } else { None }; + drop(conn); if let Some(user) = user { // Attach the `User` model from the database to the request req.mut_extensions().insert(user); diff --git a/src/tests/all.rs b/src/tests/all.rs index dae2d6fa184..b9e24a28f4f 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -274,3 +274,16 @@ fn new_category<'a>(category: &'a str, slug: &'a str, description: &'a str) -> N fn logout(req: &mut dyn Request) { req.mut_extensions().pop::(); } + +#[test] +fn multiple_live_references_to_the_same_connection_can_be_checked_out() { + use std::ptr; + + let (_bomb, app, _) = app(); + let conn1 = app.diesel_database.get().unwrap(); + let conn2 = app.diesel_database.get().unwrap(); + let conn1_ref: &PgConnection = &conn1; + let conn2_ref: &PgConnection = &conn2; + + assert!(ptr::eq(conn1_ref, conn2_ref)); +} diff --git a/src/tests/util.rs b/src/tests/util.rs index 7af49eec111..dab30a09527 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -29,6 +29,7 @@ use cargo_registry::{ uploaders::Uploader, App, }; +use diesel::PgConnection; use std::{rc::Rc, sync::Arc}; use conduit::{Handler, Method, Request}; @@ -73,7 +74,7 @@ impl TestApp { /// Within each test, the connection pool only has 1 connection so it is necessary to drop the /// connection before making any API calls. Once the closure returns, the connection is /// dropped, ensuring it is returned to the pool and available for any future API calls. - pub fn db T>(&self, f: F) -> T { + pub fn db T>(&self, f: F) -> T { let conn = self.0.app.diesel_database.get().unwrap(); f(&conn) } @@ -321,8 +322,6 @@ pub struct Bad { pub errors: Vec, } -pub type DieselConnection = - diesel::r2d2::PooledConnection>; type ResponseResult = Result>; /// A type providing helper methods for working with responses From f7b2d76480a08056b5052f894baa500dee4934bf Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Thu, 31 Jan 2019 16:58:42 -0700 Subject: [PATCH 2/2] Add a state method to `Pool` This is needed on master --- src/db.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/db.rs b/src/db.rs index 33503bea7e0..c70466b1643 100644 --- a/src/db.rs +++ b/src/db.rs @@ -25,6 +25,13 @@ impl DieselPool { } } + pub fn state(&self) -> r2d2::State { + match self { + DieselPool::Pool(pool) => pool.state(), + DieselPool::Test(_) => panic!("Cannot get the state of a test pool"), + } + } + fn test_conn(conn: PgConnection) -> Self { DieselPool::Test(ReentrantMutex::new(conn)) }