Skip to content

Commit e66bfda

Browse files
committed
Unconditionally update crate owners
Don't execute updates in a loop. This could lead to performance issues when a large quantities of crates are being updated. Also only operate on crate owners of type 'User'.
1 parent 5d0b2e4 commit e66bfda

File tree

1 file changed

+37
-12
lines changed
  • src/controllers/user

1 file changed

+37
-12
lines changed

src/controllers/user/me.rs

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
use std::collections::HashMap;
2+
13
use crate::controllers::prelude::*;
24

35
use crate::controllers::helpers::*;
46
use crate::email;
57
use crate::util::bad_request;
68
use crate::util::errors::CargoError;
79

8-
use crate::models::{CrateOwner, Email, Follow, NewEmail, User, Version};
10+
use crate::models::{CrateOwner, Email, Follow, NewEmail, OwnerKind, User, Version};
911
use crate::schema::{crate_owners, crates, emails, follows, users, versions};
1012
use crate::views::{EncodableMe, EncodableVersion, OwnedCrate};
1113

@@ -39,6 +41,7 @@ pub fn me(req: &mut dyn Request) -> CargoResult<Response> {
3941
let owned_crates = crate_owners::table
4042
.inner_join(crates::table)
4143
.filter(crate_owners::owner_id.eq(user_id))
44+
.filter(crate_owners::owner_kind.eq(OwnerKind::User as i32))
4245
.select((crates::id, crates::name, crate_owners::email_notifications))
4346
.order(crates::name.asc())
4447
.load(&*conn)?
@@ -231,7 +234,8 @@ pub fn regenerate_token_and_send(req: &mut dyn Request) -> CargoResult<Response>
231234
/// Handles `PUT /me/email_notifications` route
232235
pub fn update_email_notifications(req: &mut dyn Request) -> CargoResult<Response> {
233236
use self::crate_owners::dsl::*;
234-
use diesel::update;
237+
// use diesel::update;
238+
use diesel::pg::upsert::excluded;
235239

236240
#[derive(Deserialize)]
237241
struct CrateEmailNotifications {
@@ -241,20 +245,41 @@ pub fn update_email_notifications(req: &mut dyn Request) -> CargoResult<Response
241245

242246
let mut body = String::new();
243247
req.body().read_to_string(&mut body)?;
244-
let updates: Vec<CrateEmailNotifications> =
245-
serde_json::from_str(&body).map_err(|_| human("invalid json request"))?;
248+
let updates: HashMap<i32, bool> = serde_json::from_str::<Vec<CrateEmailNotifications>>(&body)
249+
.map_err(|_| human("invalid json request"))?
250+
.iter()
251+
.map(|c| (c.id, c.email_notifications))
252+
.collect();
246253

247254
let user = req.user()?;
248255
let conn = req.db_conn()?;
249256

250-
for to_update in updates {
251-
update(CrateOwner::belonging_to(user))
252-
.set(email_notifications.eq(to_update.email_notifications))
253-
.filter(crate_id.eq(to_update.id))
254-
.filter(email_notifications.ne(to_update.email_notifications))
255-
.execute(&*conn)
256-
.map_err(|_| human("Error updating crate owner email notifications"))?;
257-
}
257+
// Build inserts from existing crates beloning to the current user
258+
let to_insert = CrateOwner::belonging_to(user)
259+
.select((crate_id, owner_id, owner_kind, email_notifications))
260+
.filter(owner_kind.eq(OwnerKind::User as i32))
261+
.load(&*conn)?
262+
.into_iter()
263+
.map(
264+
|(c_id, o_id, o_kind, e_notifications): (i32, i32, i32, bool)| {
265+
(
266+
crate_id.eq(c_id),
267+
owner_id.eq(o_id),
268+
owner_kind.eq(o_kind),
269+
email_notifications
270+
.eq(updates.get(&c_id).unwrap_or(&e_notifications).to_owned()),
271+
)
272+
},
273+
)
274+
.collect::<Vec<_>>();
275+
276+
// Upsert crate owners; this should only actually exectute updates
277+
diesel::insert_into(crate_owners)
278+
.values(&to_insert)
279+
.on_conflict((crate_id, owner_id, owner_kind))
280+
.do_update()
281+
.set(email_notifications.eq(excluded(email_notifications)))
282+
.execute(&*conn)?;
258283

259284
#[derive(Serialize)]
260285
struct R {

0 commit comments

Comments
 (0)