Explicitly share a single connection in tests

[sgrif]

This is required to move forward on #1588, I have spun it off into a standalone PR for ease of review, since it's isolated and a change that makes sense on its own. This requires #1609 and includes those commits

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<PgConnection>. 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.

[sgrif]

A few hundred lines down we need &mut req, so we need to separate the connection from the request (app was already cloned in this function for other places that have similar problems)

[jtgeibel]

This looks good to me overall. For some reason Percy is showing 2 UI changes. I assumed this was an intermittent issue and started a rebuild on stable. It looks like that rebuild picked up recently merged changes and this conflicts with the logging of the connection pool status logging:

error[E0599]: no method named `state` found for type `db::DieselPool` in the current scope
  --> src/middleware/log_connection_pool_status.rs:38:42
   |
38 |                 self.app.diesel_database.state(),
   |                                          ^^^^^
   | 
  ::: src/db.rs:15:1
   |
15 | pub enum DieselPool {
   | ------------------- method `state` not found for this

I'm going to test on staging to see if the Percy errors are legit or not.

[sgrif]

The changes looks like something on Percy's end. The old version didn't render the downloads graph and the new one does

[sgrif]

Fixed the conflicts w/ master

[jtgeibel]

Looks good. I've accepted the changes in Percy.

@bors r+

[bors]

📌 Commit f7b2d76 has been approved by jtgeibel

[bors]

⌛ Testing commit f7b2d76 with merge e5c60e2...

[bors]

☀️ Test successful - checks-travis
Approved by: jtgeibel
Pushing e5c60e2 to master...