Skip to content

Missed a conversion from UserNoEmailType to User #1927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/bin/transfer-crates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use cargo_registry::{
db,
models::{Crate, OwnerKind, User},
models::{user, Crate, OwnerKind, User},
schema::{crate_owners, crates, users},
};
use std::{
Expand All @@ -16,6 +16,7 @@ use std::{
process::exit,
};

use cargo_registry::models::user::UserNoEmailType;
use diesel::prelude::*;

fn main() {
Expand Down Expand Up @@ -44,12 +45,16 @@ fn transfer(conn: &PgConnection) {
};

let from = users::table
.select(user::ALL_COLUMNS)
.filter(users::gh_login.eq(from))
.first::<User>(conn)
.first::<UserNoEmailType>(conn)
.map(User::from)
.unwrap();
let to = users::table
.select(user::ALL_COLUMNS)
.filter(users::gh_login.eq(to))
.first::<User>(conn)
.first::<UserNoEmailType>(conn)
.map(User::from)
.unwrap();

if from.gh_id != to.gh_id {
Expand Down
22 changes: 12 additions & 10 deletions src/controllers/krate/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
//! `Cargo.toml` file.

use crate::controllers::prelude::*;
use crate::models::user;
use crate::models::user::UserNoEmailType;
use crate::models::{
Category, Crate, CrateCategory, CrateKeyword, CrateVersions, Keyword, RecentCrateDownloads,
User, Version,
Version,
};
use crate::schema::*;
use crate::views::{
Expand Down Expand Up @@ -105,10 +107,10 @@ pub fn show(req: &mut dyn Request) -> CargoResult<Response> {
let conn = req.db_conn()?;
let krate = Crate::by_name(name).first::<Crate>(&*conn)?;

let mut versions_and_publishers: Vec<(Version, Option<User>)> = krate
let mut versions_and_publishers: Vec<(Version, Option<UserNoEmailType>)> = krate
.all_versions()
.left_outer_join(users::table)
.select((versions::all_columns, users::all_columns.nullable()))
.select((versions::all_columns, user::ALL_COLUMNS.nullable()))
.load(&*conn)?;
versions_and_publishers.sort_by(|a, b| b.0.num.cmp(&a.0.num));
let ids = versions_and_publishers.iter().map(|v| v.0.id).collect();
Expand Down Expand Up @@ -151,7 +153,7 @@ pub fn show(req: &mut dyn Request) -> CargoResult<Response> {
),
versions: versions_and_publishers
.into_iter()
.map(|(v, pb)| v.encodable(&krate.name, pb))
.map(|(v, pb)| v.encodable(&krate.name, pb.map(From::from)))
.collect(),
keywords: kws.into_iter().map(Keyword::encodable).collect(),
categories: cats.into_iter().map(Category::encodable).collect(),
Expand Down Expand Up @@ -187,15 +189,15 @@ pub fn versions(req: &mut dyn Request) -> CargoResult<Response> {
let crate_name = &req.params()["crate_id"];
let conn = req.db_conn()?;
let krate = Crate::by_name(crate_name).first::<Crate>(&*conn)?;
let mut versions_and_publishers: Vec<(Version, Option<User>)> = krate
let mut versions_and_publishers: Vec<(Version, Option<UserNoEmailType>)> = krate
.all_versions()
.left_outer_join(users::table)
.select((versions::all_columns, users::all_columns.nullable()))
.select((versions::all_columns, user::ALL_COLUMNS.nullable()))
.load(&*conn)?;
versions_and_publishers.sort_by(|a, b| b.0.num.cmp(&a.0.num));
let versions = versions_and_publishers
.into_iter()
.map(|(v, pb)| v.encodable(crate_name, pb))
.map(|(v, pb)| v.encodable(crate_name, pb.map(From::from)))
.collect();

#[derive(Serialize)]
Expand Down Expand Up @@ -227,11 +229,11 @@ pub fn reverse_dependencies(req: &mut dyn Request) -> CargoResult<Response> {
.select((
versions::all_columns,
crates::name,
users::all_columns.nullable(),
user::ALL_COLUMNS.nullable(),
))
.load::<(Version, String, Option<User>)>(&*conn)?
.load::<(Version, String, Option<UserNoEmailType>)>(&*conn)?
.into_iter()
.map(|(version, krate_name, published_by)| version.encodable(&krate_name, published_by))
.map(|(version, krate_name, user)| version.encodable(&krate_name, user.map(From::from)))
.collect();

#[derive(Serialize)]
Expand Down
24 changes: 13 additions & 11 deletions src/controllers/user/me.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::email;
use crate::util::bad_request;
use crate::util::errors::CargoError;

use crate::models::user::{UserNoEmailType, ALL_COLUMNS};
use crate::models::{CrateOwner, Email, Follow, NewEmail, OwnerKind, User, Version};
use crate::schema::{crate_owners, crates, emails, follows, users, versions};
use crate::views::{EncodableMe, EncodableVersion, OwnedCrate};
Expand All @@ -31,12 +32,12 @@ pub fn me(req: &mut dyn Request) -> CargoResult<Response> {
.find(user_id)
.left_join(emails::table)
.select((
users::all_columns,
ALL_COLUMNS,
emails::verified.nullable(),
emails::email.nullable(),
emails::token_generated_at.nullable().is_not_null(),
))
.first::<(User, Option<bool>, Option<String>, bool)>(&*conn)?;
.first::<(UserNoEmailType, Option<bool>, Option<String>, bool)>(&*conn)?;

let owned_crates = crate_owners::table
.inner_join(crates::table)
Expand All @@ -55,10 +56,13 @@ pub fn me(req: &mut dyn Request) -> CargoResult<Response> {

let verified = verified.unwrap_or(false);
let verification_sent = verified || verification_sent;
let user = User { email, ..user };
// PR :: https://github.com/rust-lang/crates.io/pull/1891
// Will modify this so that we don't need this kind of conversion anymore...
// In fact, the PR will use the email that we obtained from the above SQL queries
// and pass it along the encodable_private function below.

Ok(req.json(&EncodableMe {
user: user.encodable_private(verified, verification_sent),
user: User::from(user).encodable_private(email, verified, verification_sent),
owned_crates,
}))
}
Expand All @@ -76,19 +80,17 @@ pub fn updates(req: &mut dyn Request) -> CargoResult<Response> {
.left_outer_join(users::table)
.filter(crates::id.eq(any(followed_crates)))
.order(versions::created_at.desc())
.select((
versions::all_columns,
crates::name,
users::all_columns.nullable(),
))
.select((versions::all_columns, crates::name, ALL_COLUMNS.nullable()))
.paginate(&req.query())?
.load::<(Version, String, Option<User>)>(&*conn)?;
.load::<(Version, String, Option<UserNoEmailType>)>(&*conn)?;

let more = data.next_page_params().is_some();

let versions = data
.into_iter()
.map(|(version, crate_name, published_by)| version.encodable(&crate_name, published_by))
.map(|(version, crate_name, published_by)| {
version.encodable(&crate_name, published_by.map(From::from))
})
.collect();

#[derive(Serialize)]
Expand Down
7 changes: 5 additions & 2 deletions src/controllers/user/other.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::controllers::prelude::*;

use crate::models::user;
use crate::models::user::UserNoEmailType;
use crate::models::{OwnerKind, User};
use crate::schema::{crate_owners, crates, users};
use crate::views::EncodablePublicUser;
Expand All @@ -11,16 +13,17 @@ pub fn show(req: &mut dyn Request) -> CargoResult<Response> {
let name = &req.params()["user_id"].to_lowercase();
let conn = req.db_conn()?;
let user = users
.select(user::ALL_COLUMNS)
.filter(crate::lower(gh_login).eq(name))
.order(id.desc())
.first::<User>(&*conn)?;
.first::<UserNoEmailType>(&*conn)?;

#[derive(Serialize)]
struct R {
user: EncodablePublicUser,
}
Ok(req.json(&R {
user: user.encodable_public(),
user: User::from(user).encodable_public(),
}))
}

Expand Down
9 changes: 6 additions & 3 deletions src/controllers/user/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use crate::github;
use conduit_cookie::RequestSession;
use oauth2::{prelude::*, AuthorizationCode, TokenResponse};

use crate::models::user;
use crate::models::user::UserNoEmailType;
use crate::models::{NewUser, User};
use crate::schema::users;
use crate::util::errors::{CargoError, ReadOnlyMode};
Expand Down Expand Up @@ -130,10 +132,11 @@ impl GithubUser {
// just look for an existing user
if e.is::<ReadOnlyMode>() {
users::table
.select(user::ALL_COLUMNS)
.filter(users::gh_id.eq(self.id))
.first(conn)
.optional()?
.ok_or(e)
.first::<UserNoEmailType>(conn)
.map(User::from)
.map_err(|_| e)
} else {
Err(e)
}
Expand Down
18 changes: 11 additions & 7 deletions src/controllers/version/deprecated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@

use crate::controllers::prelude::*;

use crate::models::{Crate, User, Version};
use crate::models::user;
use crate::models::user::UserNoEmailType;
use crate::models::{Crate, Version};
use crate::schema::*;
use crate::views::EncodableVersion;

Expand All @@ -28,12 +30,14 @@ pub fn index(req: &mut dyn Request) -> CargoResult<Response> {
.select((
versions::all_columns,
crates::name,
users::all_columns.nullable(),
user::ALL_COLUMNS.nullable(),
))
.filter(versions::id.eq(any(ids)))
.load::<(Version, String, Option<User>)>(&*conn)?
.load::<(Version, String, Option<UserNoEmailType>)>(&*conn)?
.into_iter()
.map(|(version, crate_name, published_by)| version.encodable(&crate_name, published_by))
.map(|(version, crate_name, published_by)| {
version.encodable(&crate_name, published_by.map(From::from))
})
.collect();

#[derive(Serialize)]
Expand All @@ -50,14 +54,14 @@ pub fn show_by_id(req: &mut dyn Request) -> CargoResult<Response> {
let id = &req.params()["version_id"];
let id = id.parse().unwrap_or(0);
let conn = req.db_conn()?;
let (version, krate, published_by): (Version, Crate, Option<User>) = versions::table
let (version, krate, published_by): (Version, Crate, Option<UserNoEmailType>) = versions::table
.find(id)
.inner_join(crates::table)
.left_outer_join(users::table)
.select((
versions::all_columns,
crate::models::krate::ALL_COLUMNS,
users::all_columns.nullable(),
user::ALL_COLUMNS.nullable(),
))
.first(&*conn)?;

Expand All @@ -66,6 +70,6 @@ pub fn show_by_id(req: &mut dyn Request) -> CargoResult<Response> {
version: EncodableVersion,
}
Ok(req.json(&R {
version: version.encodable(&krate.name, published_by),
version: version.encodable(&krate.name, published_by.map(From::from)),
}))
}
7 changes: 6 additions & 1 deletion src/middleware/current_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use diesel::prelude::*;
use crate::db::RequestTransaction;
use crate::util::errors::{std_error, CargoResult, ChainError, Unauthorized};

use crate::models::user::{UserNoEmailType, ALL_COLUMNS};
use crate::models::User;
use crate::schema::users;

Expand All @@ -31,9 +32,13 @@ 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::<User>(&*conn);
let maybe_user = users::table
.select(ALL_COLUMNS)
.find(id)
.first::<UserNoEmailType>(&*conn);
drop(conn);
if let Ok(user) = maybe_user {
let user = User::from(user);
// Attach the `User` model from the database to the request
req.mut_extensions().insert(user);
req.mut_extensions()
Expand Down
2 changes: 1 addition & 1 deletion src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,5 @@ mod owner;
mod rights;
mod team;
mod token;
mod user;
pub mod user;
mod version;
7 changes: 5 additions & 2 deletions src/models/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use url::Url;

use crate::app::App;
use crate::email;
use crate::models::user;
use crate::models::user::UserNoEmailType;
use crate::util::{human, CargoResult};

use crate::models::{
Expand Down Expand Up @@ -399,9 +401,10 @@ impl Crate {
let users = CrateOwner::by_owner_kind(OwnerKind::User)
.filter(crate_owners::crate_id.eq(self.id))
.inner_join(users::table)
.select(users::all_columns)
.load(conn)?
.select(user::ALL_COLUMNS)
.load::<UserNoEmailType>(conn)?
.into_iter()
.map(User::from)
.map(Owner::User);
let teams = CrateOwner::by_owner_kind(OwnerKind::Team)
.filter(crate_owners::crate_id.eq(self.id))
Expand Down
5 changes: 4 additions & 1 deletion src/models/owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::app::App;
use crate::github;
use crate::util::{human, CargoResult};

use crate::models::user::{UserNoEmailType, ALL_COLUMNS};
use crate::models::{Crate, Team, User};
use crate::schema::{crate_owners, users};
use crate::views::EncodableOwner;
Expand Down Expand Up @@ -71,8 +72,10 @@ impl Owner {
)?))
} else {
users::table
.select(ALL_COLUMNS)
.filter(users::gh_login.eq(name))
.first(conn)
.first::<UserNoEmailType>(conn)
.map(User::from)
.map(Owner::User)
.map_err(|_| human(&format_args!("could not find user with login `{}`", name)))
}
Expand Down
Loading