diff --git a/src/bin/update-downloads.rs b/src/bin/update-downloads.rs index b477ed7ffb1..e28a7512355 100644 --- a/src/bin/update-downloads.rs +++ b/src/bin/update-downloads.rs @@ -149,9 +149,7 @@ mod test { user_id, ) .unwrap(); - let version = version - .save(conn, &[], Some("someone@example.com".into())) - .unwrap(); + let version = version.save(conn, &[], "someone@example.com").unwrap(); (krate, version) } diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index 4d5fd4e19b8..55312df7b0e 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -3,8 +3,6 @@ use std::collections::HashMap; use std::sync::Arc; -use chrono::offset::TimeZone; -use chrono::{DateTime, Utc}; use hex::ToHex; use crate::git; @@ -67,14 +65,13 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { let conn = app.diesel_database.get()?; - let mut other_warnings = vec![]; let verified_email_address = user.verified_email(&conn)?; - - // This function can be inlined (with only the error-returning functionality) and its unit - // tests deleted after 2019-02-28; it was created to make injecting the date for tests easier. - // The integration tests in src/tests/krate.rs cover the current production behavior (and will - // need to be updated at that time) - verified_email_check(&mut other_warnings, &verified_email_address, Utc::now())?; + let verified_email_address = verified_email_address.ok_or_else(|| { + human( + "A verified email address is required to publish crates to crates.io. \ + Visit https://crates.io/me to set and verify your email address.", + ) + })?; // Create a transaction on the database, if there are no errors, // commit the transactions to record a new or updated crate. @@ -150,7 +147,7 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { file_length as i32, user.id, )? - .save(&conn, &new_crate.authors, verified_email_address)?; + .save(&conn, &new_crate.authors, &verified_email_address)?; // Link this new version to all dependencies let git_deps = dependency::add_dependencies(&conn, &new_crate.deps, version.id)?; @@ -210,10 +207,13 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { crate_bomb.path = None; readme_bomb.path = None; + // The `other` field on `PublishWarnings` was introduced to handle a temporary warning + // that is no longer needed. As such, crates.io currently does not return any `other` + // warnings at this time, but if we need to, the field is available. let warnings = PublishWarnings { invalid_categories: ignored_invalid_categories, invalid_badges: ignored_invalid_badges, - other: other_warnings, + other: vec![], }; Ok(req.json(&GoodCrate { @@ -268,97 +268,3 @@ fn parse_new_headers(req: &mut dyn Request) -> CargoResult<(EncodableCrateUpload let user = req.user()?; Ok((new, user.clone())) } - -fn verified_email_check( - other_warnings: &mut Vec, - verified_email_address: &Option, - now: DateTime, -) -> CargoResult<()> { - match verified_email_address { - Some(_) => Ok(()), - None => { - if now < Utc.ymd(2019, 3, 1).and_hms(0, 0, 0) { - other_warnings.push(String::from( - "You do not currently have a verified email address associated with your \ - crates.io account. Starting 2019-02-28, a verified email will be required to \ - publish crates. Visit https://crates.io/me to set and verify your email \ - address.", - )); - Ok(()) - } else { - Err(human( - "A verified email address is required to publish crates to crates.io. \ - Visit https://crates.io/me to set and verify your email address.", - )) - } - } - } -} - -// These tests should be deleted after 2018-02-28; this functionality will then be covered by -// integration tests in src/tests/krate.rs. -#[cfg(test)] -mod tests { - use super::*; - use chrono::offset::TimeZone; - - #[test] - fn allow_publish_with_verified_email_without_warning_before_2018_02_28() { - let mut warnings = vec![]; - - let fake_current_date = Utc.ymd(2019, 2, 27).and_hms(0, 0, 0); - let result = verified_email_check( - &mut warnings, - &Some("someone@example.com".into()), - fake_current_date, - ); - - assert!(result.is_ok()); - assert_eq!(warnings.len(), 0); - } - - #[test] - fn allow_publish_with_verified_email_without_error_after_2018_02_28() { - let mut warnings = vec![]; - - let fake_current_date = Utc.ymd(2019, 3, 1).and_hms(0, 0, 0); - let result = verified_email_check( - &mut warnings, - &Some("someone@example.com".into()), - fake_current_date, - ); - - assert!(result.is_ok()); - assert_eq!(warnings.len(), 0); - } - - #[test] - fn warn_without_verified_email_before_2018_02_28() { - let mut warnings = vec![]; - - let fake_current_date = Utc.ymd(2019, 2, 27).and_hms(0, 0, 0); - let result = verified_email_check(&mut warnings, &None, fake_current_date); - - assert!(result.is_ok()); - assert_eq!(warnings.len(), 1); - assert_eq!(warnings[0], "You do not currently have a verified email address associated \ - with your crates.io account. Starting 2019-02-28, a verified email will be required to \ - publish crates. Visit https://crates.io/me to set and verify your email address."); - } - - #[test] - fn error_without_verified_email_after_2018_02_28() { - let mut warnings = vec![]; - - let fake_current_date = Utc.ymd(2019, 3, 1).and_hms(0, 0, 0); - let result = verified_email_check(&mut warnings, &None, fake_current_date); - - assert!(result.is_err()); - assert_eq!( - result.err().unwrap().description(), - "A verified email address is required to \ - publish crates to crates.io. Visit https://crates.io/me to set and verify your email \ - address." - ); - } -} diff --git a/src/models/version.rs b/src/models/version.rs index 1e61c8a04b0..202bbcec62a 100644 --- a/src/models/version.rs +++ b/src/models/version.rs @@ -148,12 +148,11 @@ impl NewVersion { Ok(new_version) } - // TODO: change `published_by_email` to be `String` after 2019-02-28 pub fn save( &self, conn: &PgConnection, authors: &[String], - published_by_email: Option, + published_by_email: &str, ) -> CargoResult { use crate::schema::version_authors::{name, version_id}; use crate::schema::versions::dsl::*; @@ -176,15 +175,12 @@ impl NewVersion { .values(self) .get_result::(conn)?; - // TODO: Remove the `if` around this insert after 2019-02-28 - if let Some(published_by_email) = published_by_email { - insert_into(versions_published_by::table) - .values(( - versions_published_by::version_id.eq(version.id), - versions_published_by::email.eq(published_by_email), - )) - .execute(conn)?; - } + insert_into(versions_published_by::table) + .values(( + versions_published_by::version_id.eq(version.id), + versions_published_by::email.eq(published_by_email), + )) + .execute(conn)?; let new_authors = authors .iter() diff --git a/src/tests/builders.rs b/src/tests/builders.rs index 93f2049f520..6dac66c995d 100644 --- a/src/tests/builders.rs +++ b/src/tests/builders.rs @@ -90,7 +90,7 @@ impl<'a> VersionBuilder<'a> { self.size, published_by, )? - .save(connection, &[], Some("someone@example.com".into()))?; + .save(connection, &[], "someone@example.com")?; if self.yanked { vers = update(&vers) diff --git a/src/tests/http-data/krate_new_krate_with_verified_email_doesnt_warn b/src/tests/http-data/krate_new_krate_records_verified_email similarity index 100% rename from src/tests/http-data/krate_new_krate_with_verified_email_doesnt_warn rename to src/tests/http-data/krate_new_krate_records_verified_email diff --git a/src/tests/http-data/krate_new_krate_with_unverified_email_warns b/src/tests/http-data/krate_new_krate_with_unverified_email_warns deleted file mode 100644 index f3d39f24850..00000000000 --- a/src/tests/http-data/krate_new_krate_with_unverified_email_warns +++ /dev/null @@ -1,73 +0,0 @@ -[ - { - "request": { - "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo_unverified_email/foo_unverified_email-1.0.0.crate", - "method": "PUT", - "headers": [ - [ - "accept", - "*/*" - ], - [ - "content-length", - "35" - ], - [ - "host", - "alexcrichton-test.s3.amazonaws.com" - ], - [ - "accept-encoding", - "gzip" - ], - [ - "user-agent", - "reqwest/0.9.1" - ], - [ - "content-type", - "application/x-tar" - ], - [ - "authorization", - "AWS AKIAICL5IWUZYWWKA7JA:uDc39eNdF6CcwB+q+JwKsoDLQc4=" - ], - [ - "date", - "Fri, 15 Sep 2017 07:53:06 -0700" - ] - ], - "body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA=" - }, - "response": { - "status": 200, - "headers": [ - [ - "x-amz-request-id", - "26589A5E52F8395C" - ], - [ - "ETag", - "\"f9016ad360cebb4fe2e6e96e5949f022\"" - ], - [ - "date", - "Fri, 15 Sep 2017 14:53:07 GMT" - ], - [ - "content-length", - "0" - ], - [ - "x-amz-id-2", - "JdIvnNTw53aqXjBIqBLNuN4kxf/w1XWX+xuIiGBDYy7yzOSDuAMtBSrTW4ZWetcCIdqCUHuQ51A=" - ], - [ - "Server", - "AmazonS3" - ] - ], - "body": "" - } - } -] \ No newline at end of file diff --git a/src/tests/http-data/krate_new_krate_without_any_email_warns b/src/tests/http-data/krate_new_krate_without_any_email_warns deleted file mode 100644 index 320600c9030..00000000000 --- a/src/tests/http-data/krate_new_krate_without_any_email_warns +++ /dev/null @@ -1,73 +0,0 @@ -[ - { - "request": { - "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo_no_email/foo_no_email-1.0.0.crate", - "method": "PUT", - "headers": [ - [ - "accept", - "*/*" - ], - [ - "content-length", - "35" - ], - [ - "host", - "alexcrichton-test.s3.amazonaws.com" - ], - [ - "accept-encoding", - "gzip" - ], - [ - "user-agent", - "reqwest/0.9.1" - ], - [ - "content-type", - "application/x-tar" - ], - [ - "authorization", - "AWS AKIAICL5IWUZYWWKA7JA:uDc39eNdF6CcwB+q+JwKsoDLQc4=" - ], - [ - "date", - "Fri, 15 Sep 2017 07:53:06 -0700" - ] - ], - "body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA=" - }, - "response": { - "status": 200, - "headers": [ - [ - "x-amz-request-id", - "26589A5E52F8395C" - ], - [ - "ETag", - "\"f9016ad360cebb4fe2e6e96e5949f022\"" - ], - [ - "date", - "Fri, 15 Sep 2017 14:53:07 GMT" - ], - [ - "content-length", - "0" - ], - [ - "x-amz-id-2", - "JdIvnNTw53aqXjBIqBLNuN4kxf/w1XWX+xuIiGBDYy7yzOSDuAMtBSrTW4ZWetcCIdqCUHuQ51A=" - ], - [ - "Server", - "AmazonS3" - ] - ], - "body": "" - } - } -] \ No newline at end of file diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 4b8a1c6c067..86b490e869d 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -1073,88 +1073,59 @@ fn new_krate_with_readme() { assert_eq!(json.krate.max_version, "1.0.0"); } -// This test will start failing on 2019-02-28 and should be updated at that time. -// See https://github.com/rust-lang/crates-io-cargo-teams/issues/8 #[test] -fn new_krate_without_any_email_warns() { +fn new_krate_without_any_email_fails() { let (app, _, _, token) = TestApp::with_proxy().with_token(); + app.db(|conn| { + delete(emails::table).execute(conn).unwrap(); + }); + let crate_to_publish = PublishBuilder::new("foo_no_email"); - let json = token.publish(crate_to_publish).good(); - assert_eq!(json.warnings.other.len(), 1); - assert_eq!(json.warnings.other[0], "You do not currently have a verified email address \ - associated with your crates.io account. Starting 2019-02-28, a verified email will be required \ - to publish crates. Visit https://crates.io/me to set and verify your email address."); + let json = token.publish(crate_to_publish).bad_with_status(200); - // Don't record a verified email if there isn't one - app.db(|conn| { - let email = versions_published_by::table - .select(versions_published_by::email) - .first::(conn); - assert!(email.is_err()); - }); + assert!( + json.errors[0] + .detail + .contains("A verified email address is required to publish crates to crates.io"), + "{:?}", + json.errors + ); } -// This test will start failing on 2019-02-28 and should be updated at that time. -// See https://github.com/rust-lang/crates-io-cargo-teams/issues/8 #[test] -fn new_krate_with_unverified_email_warns() { - let (app, _, user, token) = TestApp::with_proxy().with_token(); - let user = user.as_model(); +fn new_krate_with_unverified_email_fails() { + let (app, _, _, token) = TestApp::with_proxy().with_token(); app.db(|conn| { - insert_into(emails::table) - .values(( - emails::user_id.eq(user.id), - emails::email.eq("something@example.com"), - )) + update(emails::table) + .set((emails::verified.eq(false),)) .execute(conn) .unwrap(); }); let crate_to_publish = PublishBuilder::new("foo_unverified_email"); - let json = token.publish(crate_to_publish).good(); - assert_eq!(json.warnings.other.len(), 1); - assert_eq!(json.warnings.other[0], "You do not currently have a verified email address \ - associated with your crates.io account. Starting 2019-02-28, a verified email will be required \ - to publish crates. Visit https://crates.io/me to set and verify your email address."); + let json = token.publish(crate_to_publish).bad_with_status(200); - // Don't record a verified email if there isn't one - app.db(|conn| { - let email = versions_published_by::table - .select(versions_published_by::email) - .first::(conn); - assert!(email.is_err()); - }); + assert!( + json.errors[0] + .detail + .contains("A verified email address is required to publish crates to crates.io"), + "{:?}", + json.errors + ); } #[test] -fn new_krate_with_verified_email_doesnt_warn() { - let (app, _, user, token) = TestApp::with_proxy().with_token(); - let user = user.as_model(); - - // TODO: Move this to TestApp setup for user so we don't have to do this for every test - // that publishes a crate; then edit the test for the user without a verified email to - // remove the verified email - app.db(|conn| { - insert_into(emails::table) - .values(( - emails::user_id.eq(user.id), - emails::email.eq("something@example.com"), - emails::verified.eq(true), - )) - .execute(conn) - .unwrap(); - }); +fn new_krate_records_verified_email() { + let (app, _, _, token) = TestApp::with_proxy().with_token(); let crate_to_publish = PublishBuilder::new("foo_verified_email"); - let json = token.publish(crate_to_publish).good(); - assert_eq!(json.warnings.other.len(), 0); + token.publish(crate_to_publish).good(); - // Record a verified email because there is one app.db(|conn| { let email = versions_published_by::table .select(versions_published_by::email) diff --git a/src/tests/util.rs b/src/tests/util.rs index dab30a09527..8fefe90a3c7 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -79,11 +79,28 @@ impl TestApp { f(&conn) } - /// Create a new user in the database and return a mock user session + /// Create a new user with a verified email address in the database and return a mock user + /// session /// /// This method updates the database directly - pub fn db_new_user(&self, user: &str) -> MockCookieUser { - let user = self.db(|conn| crate::new_user(user).create_or_update(conn).unwrap()); + pub fn db_new_user(&self, username: &str) -> MockCookieUser { + use cargo_registry::schema::emails; + use diesel::prelude::*; + + let user = self.db(|conn| { + let mut user = crate::new_user(username).create_or_update(conn).unwrap(); + let email = "something@example.com"; + user.email = Some(email.to_string()); + diesel::insert_into(emails::table) + .values(( + emails::user_id.eq(user.id), + emails::email.eq(email), + emails::verified.eq(true), + )) + .execute(conn) + .unwrap(); + user + }); MockCookieUser { app: TestApp(Rc::clone(&self.0)), user,