Merge branch 'master' into sg-update-swirl

Author: sgrif

Cargo.lock

@@ -62,7 +62,7 @@ export default Route.extend({
                     .get('versions')
                     .then(versions => {
                         const latestStableVersion = versions.find(version => {
-                            if (!isUnstableVersion(version.get('num'))) {
+                            if (!isUnstableVersion(version.get('num')) && !version.get('yanked')) {
                                 return version;
                             }
                         });

app/routes/crate/version.js

@@ -1,5 +1,5 @@
 import Route from '@ember/routing/route';
-import ajax from 'ember-fetch/ajax';
+import fetch from 'fetch';
 import { serializeQueryParams } from 'ember-fetch/mixins/adapter-fetch';
 
 /**
@@ -19,8 +19,9 @@ export default Route.extend({
     async beforeModel(transition) {
         try {
             let queryParams = serializeQueryParams(transition.queryParams);
-            let d = await ajax(`/authorize?${queryParams}`);
-            let item = JSON.stringify({ ok: true, data: d });
+            let resp = await fetch(`/authorize?${queryParams}`);
+            let json = await resp.json();
+            let item = JSON.stringify({ ok: resp.ok, data: json });
             if (window.opener) {
                 window.opener.github_response = item;
             }

app/routes/github-authorize.js

@@ -53,15 +53,15 @@ export default Route.extend({
             if (!response) {
                 return;
             }
-            if (!response.ok) {
-                this.flashMessages.show('Failed to log in');
-                return;
-            }
+
             let { data } = response;
-            if (data.errors) {
+            if (data && data.errors) {
                 let error = `Failed to log in: ${data.errors[0].detail}`;
                 this.flashMessages.show(error);
                 return;
+            } else if (!response.ok) {
+                this.flashMessages.show('Failed to log in');
+                return;
             }
 
             let user = this.store.push(this.store.normalize('user', data.user));

app/routes/login.js

@@ -1,9 +1,10 @@
 //! Application-wide components in a struct accessible from each request
 
-use crate::{db, util::CargoResult, Config, Env};
+use crate::{db, Config, Env};
 use std::{path::PathBuf, sync::Arc, time::Duration};
 
 use diesel::r2d2;
+use reqwest::Client;
 use scheduled_thread_pool::ScheduledThreadPool;
 
 /// The `App` struct holds the main components of the application like
@@ -26,17 +27,24 @@ pub struct App {
 
     /// The server configuration
     pub config: Config,
+
+    /// A configured client for outgoing HTTP requests
+    ///
+    /// In production this shares a single connection pool across requests.  In tests
+    /// this is either None (in which case any attempt to create an outgoing connection
+    /// will panic) or a `Client` configured with a per-test replay proxy.
+    http_client: Option<Client>,
 }
 
 impl App {
-    /// Creates a new `App` with a given `Config`
+    /// Creates a new `App` with a given `Config` and an optional HTTP `Client`
     ///
     /// Configures and sets up:
     ///
     /// - GitHub OAuth
     /// - Database connection pools
-    /// - A `git2::Repository` instance from the index repo checkout (that server.rs ensures exists)
-    pub fn new(config: &Config) -> App {
+    /// - Holds an HTTP `Client` and associated connection pool, if provided
+    pub fn new(config: &Config, http_client: Option<Client>) -> App {
         let mut github = oauth2::Config::new(
             &config.gh_client_id,
             &config.gh_client_secret,
@@ -90,19 +98,22 @@ impl App {
             session_key: config.session_key.clone(),
             git_repo_checkout: config.git_repo_checkout.clone(),
             config: config.clone(),
+            http_client,
         }
     }
 
     /// Returns a client for making HTTP requests to upload crate files.
     ///
-    /// The handle will go through a proxy if the uploader being used has specified one, which
-    /// is only done in tests with `TestApp::with_proxy()` in order to be able to record and
-    /// inspect the HTTP requests that tests make.
-    pub fn http_client(&self) -> CargoResult<reqwest::Client> {
-        let mut builder = reqwest::Client::builder();
-        if let Some(proxy) = self.config.uploader.proxy() {
-            builder = builder.proxy(reqwest::Proxy::all(proxy)?);
-        }
-        Ok(builder.build()?)
+    /// The client will go through a proxy if the application was configured via
+    /// `TestApp::with_proxy()`.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the application was not initialized with a client.  This should only occur in
+    /// tests that were not properly initialized.
+    pub fn http_client(&self) -> &Client {
+        self.http_client
+            .as_ref()
+            .expect("No HTTP client is configured.  In tests, use `TestApp::with_proxy()`.")
     }
 }

src/app.rs

@@ -21,6 +21,7 @@ use chrono::{TimeZone, Utc};
 use diesel::{dsl::any, prelude::*};
 use docopt::Docopt;
 use flate2::read::GzDecoder;
+use reqwest::Client;
 use tar::{self, Archive};
 
 const DEFAULT_PAGE_SIZE: usize = 25;
@@ -94,6 +95,8 @@ fn main() {
         total_pages + 1
     };
 
+    let client = Client::new();
+
     for (page_num, version_ids_chunk) in version_ids.chunks(page_size).enumerate() {
         println!(
             "= Page {} of {} ==================================",
@@ -117,22 +120,18 @@ fn main() {
                     krate_name, version.num
                 )
             });
+            let client = client.clone();
             let handle = thread::spawn(move || {
                 println!("[{}-{}] Rendering README...", krate_name, version.num);
-                let readme = get_readme(&config, &version, &krate_name);
+                let readme = get_readme(&config, &client, &version, &krate_name);
                 if readme.is_none() {
                     return;
                 }
                 let readme = readme.unwrap();
                 let readme_path = format!("readmes/{0}/{0}-{1}.html", krate_name, version.num);
                 config
                     .uploader
-                    .upload(
-                        &reqwest::Client::new(),
-                        &readme_path,
-                        readme.into_bytes(),
-                        "text/html",
-                    )
+                    .upload(&client, &readme_path, readme.into_bytes(), "text/html")
                     .unwrap_or_else(|_| {
                         panic!(
                             "[{}-{}] Couldn't upload file to S3",
@@ -151,12 +150,17 @@ fn main() {
 }
 
 /// Renders the readme of an uploaded crate version.
-fn get_readme(config: &Config, version: &Version, krate_name: &str) -> Option<String> {
+fn get_readme(
+    config: &Config,
+    client: &Client,
+    version: &Version,
+    krate_name: &str,
+) -> Option<String> {
     let location = config
         .uploader
-        .crate_location(krate_name, &version.num.to_string())?;
+        .crate_location(krate_name, &version.num.to_string());
 
-    let mut response = match reqwest::get(&location) {
+    let mut response = match client.get(&location).send() {
         Ok(r) => r,
         Err(err) => {
             println!(

src/bin/render-readmes.rs

@@ -9,6 +9,7 @@ use std::{
 
 use civet::Server as CivetServer;
 use conduit_hyper::Service as HyperService;
+use reqwest::Client;
 
 enum Server {
     Civet(CivetServer),
@@ -24,7 +25,9 @@ fn main() {
     env_logger::init();
 
     let config = cargo_registry::Config::default();
-    let app = App::new(&config);
+    let client = Client::new();
+
+    let app = App::new(&config, Some(client));
     let app = cargo_registry::build_handler(Arc::new(app));
 
     // On every server restart, ensure the categories available in the database match

src/bin/server.rs

@@ -68,7 +68,6 @@ impl Default for Config {
                         &api_protocol,
                     ),
                     cdn: dotenv::var("S3_CDN").ok(),
-                    proxy: None,
                 }
             }
             (Env::Production, Replica::ReadOnlyMirror) => {
@@ -89,7 +88,6 @@ impl Default for Config {
                         &api_protocol,
                     ),
                     cdn: dotenv::var("S3_CDN").ok(),
-                    proxy: None,
                 }
             }
             // In Development mode, either running as a primary instance or a read-only mirror
@@ -109,7 +107,6 @@ impl Default for Config {
                             &api_protocol,
                         ),
                         cdn: dotenv::var("S3_CDN").ok(),
-                        proxy: None,
                     }
                 } else {
                     // If we don't set the `S3_BUCKET` variable, we'll use a development-only

src/config.rs

@@ -169,8 +169,7 @@ pub fn readme(req: &mut dyn Request) -> CargoResult<Response> {
         .app()
         .config
         .uploader
-        .readme_location(crate_name, version)
-        .ok_or_else(|| human("crate readme not found"))?;
+        .readme_location(crate_name, version);
 
     if req.wants_json() {
         #[derive(Serialize)]