From d85ecc83023e36ffe76accfada8945b627579410 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 13 Sep 2022 18:24:40 -0700 Subject: [PATCH 01/22] Add collab APIs for new signup flow Co-authored-by: Nathan Sobo --- .../20220913211150_create_signups.sql | 25 ++ crates/collab/src/api.rs | 42 ++- crates/collab/src/db.rs | 272 ++++++++++++++++++ 3 files changed, 338 insertions(+), 1 deletion(-) create mode 100644 crates/collab/migrations/20220913211150_create_signups.sql diff --git a/crates/collab/migrations/20220913211150_create_signups.sql b/crates/collab/migrations/20220913211150_create_signups.sql new file mode 100644 index 0000000000..783cdf8c0a --- /dev/null +++ b/crates/collab/migrations/20220913211150_create_signups.sql @@ -0,0 +1,25 @@ +CREATE SEQUENCE metrics_id_seq; + +CREATE TABLE IF NOT EXISTS "signups" ( + "id" SERIAL PRIMARY KEY NOT NULL, + "email_address" VARCHAR NOT NULL, + "email_confirmation_code" VARCHAR(64) NOT NULL, + "email_confirmation_sent" BOOLEAN NOT NULL, + "metrics_id" INTEGER NOT NULL DEFAULT nextval('metrics_id_seq'), + "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + "user_id" INTEGER REFERENCES users (id), + + "platform_mac" BOOLEAN NOT NULL, + "platform_linux" BOOLEAN NOT NULL, + "platform_windows" BOOLEAN NOT NULL, + "platform_unknown" BOOLEAN NOT NULL, + + "editor_features" VARCHAR[] NOT NULL, + "programming_languages" VARCHAR[] NOT NULL +); + +CREATE UNIQUE INDEX "index_signups_on_email_address" ON "signups" ("email_address"); +CREATE INDEX "index_signups_on_email_confirmation_sent" ON "signups" ("email_confirmation_sent"); + +ALTER TABLE "users" + ADD "metrics_id" INTEGER DEFAULT nextval('metrics_id_seq'); diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index eafeae0864..42825d25ff 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -1,6 +1,6 @@ use crate::{ auth, - db::{ProjectId, User, UserId}, + db::{ProjectId, Signup, SignupInvite, SignupRedemption, User, UserId}, rpc::{self, ResultExt}, AppState, Error, Result, }; @@ -45,6 +45,10 @@ pub fn routes(rpc_server: &Arc, state: Arc) -> Router Result> { Ok(Json(app.db.get_user_for_invite_code(&code).await?)) } + +async fn create_signup( + Json(params): Json, + Extension(app): Extension>, +) -> Result<()> { + app.db.create_signup(params).await?; + Ok(()) +} + +async fn redeem_signup( + Json(redemption): Json, + Extension(app): Extension>, +) -> Result<()> { + app.db.redeem_signup(redemption).await?; + Ok(()) +} + +async fn record_signup_invites_sent( + Json(params): Json>, + Extension(app): Extension>, +) -> Result<()> { + app.db.record_signup_invites_sent(¶ms).await?; + Ok(()) +} + +#[derive(Deserialize)] +pub struct GetSignupInvitesParams { + pub count: usize, +} + +async fn get_signup_invites( + Query(params): Query, + Extension(app): Extension>, +) -> Result>> { + Ok(Json(app.db.get_signup_invites(params.count).await?)) +} diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index eeb598413e..86dca6de98 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -30,6 +30,11 @@ pub trait Db: Send + Sync { async fn set_user_connected_once(&self, id: UserId, connected_once: bool) -> Result<()>; async fn destroy_user(&self, id: UserId) -> Result<()>; + async fn create_signup(&self, signup: Signup) -> Result<()>; + async fn get_signup_invites(&self, count: usize) -> Result>; + async fn record_signup_invites_sent(&self, signups: &[SignupInvite]) -> Result<()>; + async fn redeem_signup(&self, redemption: SignupRedemption) -> Result; + async fn set_invite_count(&self, id: UserId, count: u32) -> Result<()>; async fn get_invite_code_for_user(&self, id: UserId) -> Result>; async fn get_user_for_invite_code(&self, code: &str) -> Result; @@ -333,6 +338,125 @@ impl Db for PostgresDb { .map(drop)?) } + // signups + + async fn create_signup(&self, signup: Signup) -> Result<()> { + sqlx::query( + " + INSERT INTO signups + ( + email_address, + email_confirmation_code, + email_confirmation_sent, + platform_linux, + platform_mac, + platform_windows, + platform_unknown, + editor_features, + programming_languages + ) + VALUES + ($1, $2, 'f', $3, $4, $5, 'f', $6, $7) + ", + ) + .bind(&signup.email_address) + .bind(&random_email_confirmation_code()) + .bind(&signup.platform_linux) + .bind(&signup.platform_mac) + .bind(&signup.platform_windows) + .bind(&signup.editor_features) + .bind(&signup.programming_languages) + .execute(&self.pool) + .await?; + Ok(()) + } + + async fn get_signup_invites(&self, count: usize) -> Result> { + Ok(sqlx::query_as( + " + SELECT + email_address, email_confirmation_code + FROM signups + WHERE + NOT email_confirmation_sent AND + platform_mac + LIMIT $1 + ", + ) + .bind(count as i32) + .fetch_all(&self.pool) + .await?) + } + + async fn record_signup_invites_sent(&self, signups: &[SignupInvite]) -> Result<()> { + sqlx::query( + " + UPDATE signups + SET email_confirmation_sent = 't' + WHERE email_address = ANY ($1) + ", + ) + .bind( + &signups + .iter() + .map(|s| s.email_address.as_str()) + .collect::>(), + ) + .execute(&self.pool) + .await?; + Ok(()) + } + + async fn redeem_signup(&self, redemption: SignupRedemption) -> Result { + let mut tx = self.pool.begin().await?; + let signup_id: i32 = sqlx::query_scalar( + " + SELECT id + FROM signups + WHERE + email_address = $1 AND + email_confirmation_code = $2 AND + email_confirmation_sent AND + user_id is NULL + ", + ) + .bind(&redemption.email_address) + .bind(&redemption.email_confirmation_code) + .fetch_one(&mut tx) + .await?; + + let user_id: i32 = sqlx::query_scalar( + " + INSERT INTO users + (email_address, github_login, admin, invite_count, invite_code) + VALUES + ($1, $2, 'f', $3, $4) + RETURNING id + ", + ) + .bind(&redemption.email_address) + .bind(&redemption.github_login) + .bind(&redemption.invite_count) + .bind(random_invite_code()) + .fetch_one(&mut tx) + .await?; + + sqlx::query( + " + UPDATE signups + SET user_id = $1 + WHERE id = $2 + ", + ) + .bind(&user_id) + .bind(&signup_id) + .execute(&mut tx) + .await?; + + tx.commit().await?; + Ok(UserId(user_id)) + } + // invite codes async fn set_invite_count(&self, id: UserId, count: u32) -> Result<()> { @@ -1445,6 +1569,30 @@ pub struct IncomingContactRequest { pub should_notify: bool, } +#[derive(Clone, Deserialize)] +pub struct Signup { + pub email_address: String, + pub platform_mac: bool, + pub platform_windows: bool, + pub platform_linux: bool, + pub editor_features: Vec, + pub programming_languages: Vec, +} + +#[derive(FromRow, PartialEq, Debug, Serialize, Deserialize)] +pub struct SignupInvite { + pub email_address: String, + pub email_confirmation_code: String, +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct SignupRedemption { + pub email_address: String, + pub email_confirmation_code: String, + pub github_login: String, + pub invite_count: i32, +} + fn fuzzy_like_string(string: &str) -> String { let mut result = String::with_capacity(string.len() * 2 + 1); for c in string.chars() { @@ -1461,6 +1609,10 @@ fn random_invite_code() -> String { nanoid::nanoid!(16) } +fn random_email_confirmation_code() -> String { + nanoid::nanoid!(64) +} + #[cfg(test)] pub mod tests { use super::*; @@ -2400,6 +2552,105 @@ pub mod tests { ); } + #[tokio::test(flavor = "multi_thread")] + async fn test_signups() { + let postgres = TestDb::postgres().await; + let db = postgres.db(); + + // people sign up on the waitlist + for i in 0..8 { + db.create_signup(Signup { + email_address: format!("person-{i}@example.com"), + platform_mac: true, + platform_linux: true, + platform_windows: false, + editor_features: vec!["speed".into()], + programming_languages: vec!["rust".into(), "c".into()], + }) + .await + .unwrap(); + } + + // retrieve the next batch of signup emails to send + let signups_batch1 = db.get_signup_invites(3).await.unwrap(); + let addresses = signups_batch1 + .iter() + .map(|s| &s.email_address) + .collect::>(); + assert_eq!( + addresses, + &[ + "person-0@example.com", + "person-1@example.com", + "person-2@example.com" + ] + ); + assert_ne!( + signups_batch1[0].email_confirmation_code, + signups_batch1[1].email_confirmation_code + ); + + // the waitlist isn't updated until we record that the emails + // were successfully sent. + let signups_batch = db.get_signup_invites(3).await.unwrap(); + assert_eq!(signups_batch, signups_batch1); + + // once the emails go out, we can retrieve the next batch + // of signups. + db.record_signup_invites_sent(&signups_batch1) + .await + .unwrap(); + let signups_batch2 = db.get_signup_invites(3).await.unwrap(); + let addresses = signups_batch2 + .iter() + .map(|s| &s.email_address) + .collect::>(); + assert_eq!( + addresses, + &[ + "person-3@example.com", + "person-4@example.com", + "person-5@example.com" + ] + ); + + // user completes the signup process by providing their + // github account. + let user_id = db + .redeem_signup(SignupRedemption { + email_address: signups_batch1[0].email_address.clone(), + email_confirmation_code: signups_batch1[0].email_confirmation_code.clone(), + github_login: "person-0".into(), + invite_count: 5, + }) + .await + .unwrap(); + let user = db.get_user_by_id(user_id).await.unwrap().unwrap(); + assert_eq!(user.github_login, "person-0"); + assert_eq!(user.email_address.as_deref(), Some("person-0@example.com")); + assert_eq!(user.invite_count, 5); + + // cannot redeem the same signup again. + db.redeem_signup(SignupRedemption { + email_address: signups_batch1[0].email_address.clone(), + email_confirmation_code: signups_batch1[0].email_confirmation_code.clone(), + github_login: "some-other-github_account".into(), + invite_count: 5, + }) + .await + .unwrap_err(); + + // cannot redeem a signup with the wrong confirmation code. + db.redeem_signup(SignupRedemption { + email_address: signups_batch1[1].email_address.clone(), + email_confirmation_code: "the-wrong-code".to_string(), + github_login: "person-1".into(), + invite_count: 5, + }) + .await + .unwrap_err(); + } + pub struct TestDb { pub db: Option>, pub url: String, @@ -2586,6 +2837,27 @@ pub mod tests { unimplemented!() } + // signups + + async fn create_signup(&self, _signup: Signup) -> Result<()> { + unimplemented!() + } + + async fn get_signup_invites(&self, _count: usize) -> Result> { + unimplemented!() + } + + async fn record_signup_invites_sent(&self, _signups: &[SignupInvite]) -> Result<()> { + unimplemented!() + } + + async fn redeem_signup( + &self, + _redemption: SignupRedemption, + ) -> Result { + unimplemented!() + } + // invite codes async fn set_invite_count(&self, _id: UserId, _count: u32) -> Result<()> { From f8c7c925af408b223def9b44b62487fe8ab371b1 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 16 Sep 2022 12:25:42 -0700 Subject: [PATCH 02/22] Update APIs and DB interactions to reflect email confirmation step --- .../20220913211150_create_signups.down.sql | 6 + ...l => 20220913211150_create_signups.up.sql} | 6 +- crates/collab/src/api.rs | 106 +- crates/collab/src/db.rs | 1448 +++-------------- crates/collab/src/db_tests.rs | 1071 ++++++++++++ crates/collab/src/integration_tests.rs | 14 +- crates/collab/src/main.rs | 2 + crates/collab/src/rpc.rs | 41 +- 8 files changed, 1409 insertions(+), 1285 deletions(-) create mode 100644 crates/collab/migrations/20220913211150_create_signups.down.sql rename crates/collab/migrations/{20220913211150_create_signups.sql => 20220913211150_create_signups.up.sql} (81%) create mode 100644 crates/collab/src/db_tests.rs diff --git a/crates/collab/migrations/20220913211150_create_signups.down.sql b/crates/collab/migrations/20220913211150_create_signups.down.sql new file mode 100644 index 0000000000..6ef51842c9 --- /dev/null +++ b/crates/collab/migrations/20220913211150_create_signups.down.sql @@ -0,0 +1,6 @@ +DROP TABLE signups; + +ALTER TABLE users + DROP COLUMN metrics_id; + +DROP SEQUENCE metrics_id_seq; diff --git a/crates/collab/migrations/20220913211150_create_signups.sql b/crates/collab/migrations/20220913211150_create_signups.up.sql similarity index 81% rename from crates/collab/migrations/20220913211150_create_signups.sql rename to crates/collab/migrations/20220913211150_create_signups.up.sql index 783cdf8c0a..9acb313fd6 100644 --- a/crates/collab/migrations/20220913211150_create_signups.sql +++ b/crates/collab/migrations/20220913211150_create_signups.up.sql @@ -8,16 +8,18 @@ CREATE TABLE IF NOT EXISTS "signups" ( "metrics_id" INTEGER NOT NULL DEFAULT nextval('metrics_id_seq'), "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, "user_id" INTEGER REFERENCES users (id), + "inviting_user_id" INTEGER REFERENCES users (id), "platform_mac" BOOLEAN NOT NULL, "platform_linux" BOOLEAN NOT NULL, "platform_windows" BOOLEAN NOT NULL, "platform_unknown" BOOLEAN NOT NULL, - "editor_features" VARCHAR[] NOT NULL, - "programming_languages" VARCHAR[] NOT NULL + "editor_features" VARCHAR[], + "programming_languages" VARCHAR[] ); +CREATE INDEX "index_users_on_email_address" ON "users" ("email_address"); CREATE UNIQUE INDEX "index_signups_on_email_address" ON "signups" ("email_address"); CREATE INDEX "index_signups_on_email_confirmation_sent" ON "signups" ("email_confirmation_sent"); diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 42825d25ff..26521ceb27 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -1,6 +1,6 @@ use crate::{ auth, - db::{ProjectId, Signup, SignupInvite, SignupRedemption, User, UserId}, + db::{Invite, NewUserParams, ProjectId, Signup, User, UserId}, rpc::{self, ResultExt}, AppState, Error, Result, }; @@ -46,9 +46,9 @@ pub fn routes(rpc_server: &Arc, state: Arc) -> Router, - email_address: Option, - admin: bool, + email_address: String, + email_confirmation_code: Option, + invite_count: i32, } async fn create_user( @@ -123,29 +123,38 @@ async fn create_user( Extension(app): Extension>, Extension(rpc_server): Extension>, ) -> Result> { - let user_id = if let Some(invite_code) = params.invite_code { - let invitee_id = app - .db - .redeem_invite_code( - &invite_code, - ¶ms.github_login, - params.email_address.as_deref(), + let (user_id, inviter_id) = + // Creating a user via the normal signup process + if let Some(email_confirmation_code) = params.email_confirmation_code { + app.db + .create_user_from_invite( + &Invite { + email_address: params.email_address, + email_confirmation_code, + }, + NewUserParams { + github_login: params.github_login, + invite_count: params.invite_count, + }, + ) + .await? + } + // Creating a user as an admin + else { + ( + app.db + .create_user(¶ms.github_login, ¶ms.email_address, false) + .await?, + None, ) - .await?; + }; + + if let Some(inviter_id) = inviter_id { rpc_server - .invite_code_redeemed(&invite_code, invitee_id) + .invite_code_redeemed(inviter_id, user_id) .await .trace_err(); - invitee_id - } else { - app.db - .create_user( - ¶ms.github_login, - params.email_address.as_deref(), - params.admin, - ) - .await? - }; + } let user = app .db @@ -175,7 +184,9 @@ async fn update_user( } if let Some(invite_count) = params.invite_count { - app.db.set_invite_count(user_id, invite_count).await?; + app.db + .set_invite_count_for_user(user_id, invite_count) + .await?; rpc_server.invite_count_updated(user_id).await.trace_err(); } @@ -428,30 +439,39 @@ async fn create_signup( Ok(()) } -async fn redeem_signup( - Json(redemption): Json, - Extension(app): Extension>, -) -> Result<()> { - app.db.redeem_signup(redemption).await?; - Ok(()) +#[derive(Deserialize)] +pub struct CreateInviteFromCodeParams { + invite_code: String, + email_address: String, } -async fn record_signup_invites_sent( - Json(params): Json>, +async fn create_invite_from_code( + Json(params): Json, Extension(app): Extension>, -) -> Result<()> { - app.db.record_signup_invites_sent(¶ms).await?; - Ok(()) +) -> Result> { + Ok(Json( + app.db + .create_invite_from_code(¶ms.invite_code, ¶ms.email_address) + .await?, + )) } #[derive(Deserialize)] -pub struct GetSignupInvitesParams { +pub struct GetUnsentInvitesParams { pub count: usize, } -async fn get_signup_invites( - Query(params): Query, +async fn get_unsent_invites( + Query(params): Query, Extension(app): Extension>, -) -> Result>> { - Ok(Json(app.db.get_signup_invites(params.count).await?)) +) -> Result>> { + Ok(Json(app.db.get_unsent_invites(params.count).await?)) +} + +async fn record_sent_invites( + Json(params): Json>, + Extension(app): Extension>, +) -> Result<()> { + app.db.record_sent_invites(¶ms).await?; + Ok(()) } diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 86dca6de98..9c1ab84570 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1,5 +1,3 @@ -use std::{cmp, ops::Range, time::Duration}; - use crate::{Error, Result}; use anyhow::{anyhow, Context}; use async_trait::async_trait; @@ -9,6 +7,7 @@ use futures::StreamExt; use serde::{Deserialize, Serialize}; pub use sqlx::postgres::PgPoolOptions as DbOptions; use sqlx::{types::Uuid, FromRow, QueryBuilder, Row}; +use std::{cmp, ops::Range, time::Duration}; use time::{OffsetDateTime, PrimitiveDateTime}; #[async_trait] @@ -16,7 +15,7 @@ pub trait Db: Send + Sync { async fn create_user( &self, github_login: &str, - email_address: Option<&str>, + email_address: &str, admin: bool, ) -> Result; async fn get_all_users(&self, page: u32, limit: u32) -> Result>; @@ -30,20 +29,19 @@ pub trait Db: Send + Sync { async fn set_user_connected_once(&self, id: UserId, connected_once: bool) -> Result<()>; async fn destroy_user(&self, id: UserId) -> Result<()>; - async fn create_signup(&self, signup: Signup) -> Result<()>; - async fn get_signup_invites(&self, count: usize) -> Result>; - async fn record_signup_invites_sent(&self, signups: &[SignupInvite]) -> Result<()>; - async fn redeem_signup(&self, redemption: SignupRedemption) -> Result; - - async fn set_invite_count(&self, id: UserId, count: u32) -> Result<()>; + async fn set_invite_count_for_user(&self, id: UserId, count: u32) -> Result<()>; async fn get_invite_code_for_user(&self, id: UserId) -> Result>; async fn get_user_for_invite_code(&self, code: &str) -> Result; - async fn redeem_invite_code( + async fn create_invite_from_code(&self, code: &str, email_address: &str) -> Result; + + async fn create_signup(&self, signup: Signup) -> Result<()>; + async fn get_unsent_invites(&self, count: usize) -> Result>; + async fn record_sent_invites(&self, invites: &[Invite]) -> Result<()>; + async fn create_user_from_invite( &self, - code: &str, - login: &str, - email_address: Option<&str>, - ) -> Result; + invite: &Invite, + user: NewUserParams, + ) -> Result<(UserId, Option)>; /// Registers a new project for the given user. async fn register_project(&self, host_user_id: UserId) -> Result; @@ -120,8 +118,8 @@ pub trait Db: Send + Sync { max_access_token_count: usize, ) -> Result<()>; async fn get_access_token_hashes(&self, user_id: UserId) -> Result>; - #[cfg(any(test, feature = "seed-support"))] + #[cfg(any(test, feature = "seed-support"))] async fn find_org_by_slug(&self, slug: &str) -> Result>; #[cfg(any(test, feature = "seed-support"))] async fn create_org(&self, name: &str, slug: &str) -> Result; @@ -135,6 +133,7 @@ pub trait Db: Send + Sync { async fn get_accessible_channels(&self, user_id: UserId) -> Result>; async fn can_user_access_channel(&self, user_id: UserId, channel_id: ChannelId) -> Result; + #[cfg(any(test, feature = "seed-support"))] async fn add_channel_member( &self, @@ -156,10 +155,12 @@ pub trait Db: Send + Sync { count: usize, before_id: Option, ) -> Result>; + #[cfg(test)] async fn teardown(&self, url: &str); + #[cfg(test)] - fn as_fake(&self) -> Option<&tests::FakeDb>; + fn as_fake(&self) -> Option<&FakeDb>; } pub struct PostgresDb { @@ -175,6 +176,18 @@ impl PostgresDb { .context("failed to connect to postgres database")?; Ok(Self { pool }) } + + pub fn fuzzy_like_string(string: &str) -> String { + let mut result = String::with_capacity(string.len() * 2 + 1); + for c in string.chars() { + if c.is_alphanumeric() { + result.push('%'); + result.push(c); + } + } + result.push('%'); + result + } } #[async_trait] @@ -184,7 +197,7 @@ impl Db for PostgresDb { async fn create_user( &self, github_login: &str, - email_address: Option<&str>, + email_address: &str, admin: bool, ) -> Result { let query = " @@ -247,7 +260,7 @@ impl Db for PostgresDb { } async fn fuzzy_search_users(&self, name_query: &str, limit: u32) -> Result> { - let like_string = fuzzy_like_string(name_query); + let like_string = Self::fuzzy_like_string(name_query); let query = " SELECT users.* FROM users @@ -371,16 +384,16 @@ impl Db for PostgresDb { Ok(()) } - async fn get_signup_invites(&self, count: usize) -> Result> { + async fn get_unsent_invites(&self, count: usize) -> Result> { Ok(sqlx::query_as( " - SELECT - email_address, email_confirmation_code - FROM signups - WHERE - NOT email_confirmation_sent AND - platform_mac - LIMIT $1 + SELECT + email_address, email_confirmation_code + FROM signups + WHERE + NOT email_confirmation_sent AND + platform_mac + LIMIT $1 ", ) .bind(count as i32) @@ -388,16 +401,16 @@ impl Db for PostgresDb { .await?) } - async fn record_signup_invites_sent(&self, signups: &[SignupInvite]) -> Result<()> { + async fn record_sent_invites(&self, invites: &[Invite]) -> Result<()> { sqlx::query( " - UPDATE signups - SET email_confirmation_sent = 't' - WHERE email_address = ANY ($1) + UPDATE signups + SET email_confirmation_sent = 't' + WHERE email_address = ANY ($1) ", ) .bind( - &signups + &invites .iter() .map(|s| s.email_address.as_str()) .collect::>(), @@ -407,36 +420,41 @@ impl Db for PostgresDb { Ok(()) } - async fn redeem_signup(&self, redemption: SignupRedemption) -> Result { + async fn create_user_from_invite( + &self, + invite: &Invite, + user: NewUserParams, + ) -> Result<(UserId, Option)> { let mut tx = self.pool.begin().await?; - let signup_id: i32 = sqlx::query_scalar( + + let (signup_id, inviting_user_id): (i32, Option) = sqlx::query_as( " - SELECT id + SELECT id, inviting_user_id FROM signups WHERE email_address = $1 AND email_confirmation_code = $2 AND - email_confirmation_sent AND user_id is NULL ", ) - .bind(&redemption.email_address) - .bind(&redemption.email_confirmation_code) - .fetch_one(&mut tx) - .await?; + .bind(&invite.email_address) + .bind(&invite.email_confirmation_code) + .fetch_optional(&mut tx) + .await? + .ok_or_else(|| anyhow!("no such invite"))?; - let user_id: i32 = sqlx::query_scalar( + let user_id: UserId = sqlx::query_scalar( " INSERT INTO users - (email_address, github_login, admin, invite_count, invite_code) + (email_address, github_login, admin, invite_count, invite_code) VALUES - ($1, $2, 'f', $3, $4) + ($1, $2, 'f', $3, $4) RETURNING id ", ) - .bind(&redemption.email_address) - .bind(&redemption.github_login) - .bind(&redemption.invite_count) + .bind(&invite.email_address) + .bind(&user.github_login) + .bind(&user.invite_count) .bind(random_invite_code()) .fetch_one(&mut tx) .await?; @@ -453,13 +471,47 @@ impl Db for PostgresDb { .execute(&mut tx) .await?; + if let Some(inviting_user_id) = inviting_user_id { + let id: Option = sqlx::query_scalar( + " + UPDATE users + SET invite_count = invite_count - 1 + WHERE id = $1 AND invite_count > 0 + RETURNING id + ", + ) + .bind(&inviting_user_id) + .fetch_optional(&mut tx) + .await?; + + if id.is_none() { + Err(Error::Http( + StatusCode::UNAUTHORIZED, + "no invites remaining".to_string(), + ))?; + } + + sqlx::query( + " + INSERT INTO contacts + (user_id_a, user_id_b, a_to_b, should_notify, accepted) + VALUES + ($1, $2, 't', 't', 't') + ", + ) + .bind(inviting_user_id) + .bind(user_id) + .execute(&mut tx) + .await?; + } + tx.commit().await?; - Ok(UserId(user_id)) + Ok((user_id, inviting_user_id)) } // invite codes - async fn set_invite_count(&self, id: UserId, count: u32) -> Result<()> { + async fn set_invite_count_for_user(&self, id: UserId, count: u32) -> Result<()> { let mut tx = self.pool.begin().await?; if count > 0 { sqlx::query( @@ -527,83 +579,82 @@ impl Db for PostgresDb { }) } - async fn redeem_invite_code( - &self, - code: &str, - login: &str, - email_address: Option<&str>, - ) -> Result { + async fn create_invite_from_code(&self, code: &str, email_address: &str) -> Result { let mut tx = self.pool.begin().await?; - let inviter_id: Option = sqlx::query_scalar( + let existing_user: Option = sqlx::query_scalar( " - UPDATE users - SET invite_count = invite_count - 1 - WHERE - invite_code = $1 AND - invite_count > 0 - RETURNING id + SELECT id + FROM users + WHERE email_address = $1 + ", + ) + .bind(email_address) + .fetch_optional(&mut tx) + .await?; + if existing_user.is_some() { + Err(anyhow!("email address is already in use"))?; + } + + let row: Option<(UserId, i32)> = sqlx::query_as( + " + SELECT id, invite_count + FROM users + WHERE invite_code = $1 ", ) .bind(code) .fetch_optional(&mut tx) .await?; - let inviter_id = match inviter_id { - Some(inviter_id) => inviter_id, - None => { - if sqlx::query_scalar::<_, i32>("SELECT 1 FROM users WHERE invite_code = $1") - .bind(code) - .fetch_optional(&mut tx) - .await? - .is_some() - { - Err(Error::Http( - StatusCode::UNAUTHORIZED, - "no invites remaining".to_string(), - ))? - } else { - Err(Error::Http( - StatusCode::NOT_FOUND, - "invite code not found".to_string(), - ))? - } - } + let (inviter_id, invite_count) = match row { + Some(row) => row, + None => Err(Error::Http( + StatusCode::NOT_FOUND, + "invite code not found".to_string(), + ))?, }; - let invitee_id = sqlx::query_scalar( - " - INSERT INTO users - (github_login, email_address, admin, inviter_id, invite_code, invite_count) - VALUES - ($1, $2, 'f', $3, $4, $5) - RETURNING id - ", - ) - .bind(login) - .bind(email_address) - .bind(inviter_id) - .bind(random_invite_code()) - .bind(5) - .fetch_one(&mut tx) - .await - .map(UserId)?; + if invite_count == 0 { + Err(Error::Http( + StatusCode::UNAUTHORIZED, + "no invites remaining".to_string(), + ))?; + } - sqlx::query( + let email_confirmation_code: String = sqlx::query_scalar( " - INSERT INTO contacts - (user_id_a, user_id_b, a_to_b, should_notify, accepted) - VALUES - ($1, $2, 't', 't', 't') + INSERT INTO signups + ( + email_address, + email_confirmation_code, + email_confirmation_sent, + inviting_user_id, + platform_linux, + platform_mac, + platform_windows, + platform_unknown + ) + VALUES + ($1, $2, 'f', $3, 'f', 'f', 'f', 't') + ON CONFLICT (email_address) + DO UPDATE SET + inviting_user_id = excluded.inviting_user_id + RETURNING email_confirmation_code ", ) - .bind(inviter_id) - .bind(invitee_id) - .execute(&mut tx) + .bind(&email_address) + .bind(&random_email_confirmation_code()) + .bind(&inviter_id) + .fetch_one(&mut tx) .await?; tx.commit().await?; - Ok(invitee_id) + + Ok(Invite { + email_address: email_address.into(), + email_confirmation_code, + }) } // projects @@ -1418,7 +1469,7 @@ impl Db for PostgresDb { } #[cfg(test)] - fn as_fake(&self) -> Option<&tests::FakeDb> { + fn as_fake(&self) -> Option<&FakeDb> { None } } @@ -1495,19 +1546,19 @@ pub struct UserActivitySummary { #[derive(Clone, Debug, PartialEq, Serialize)] pub struct ProjectActivitySummary { - id: ProjectId, - duration: Duration, - max_collaborators: usize, + pub id: ProjectId, + pub duration: Duration, + pub max_collaborators: usize, } #[derive(Clone, Debug, PartialEq, Serialize)] pub struct UserActivityPeriod { - project_id: ProjectId, + pub project_id: ProjectId, #[serde(with = "time::serde::iso8601")] - start: OffsetDateTime, + pub start: OffsetDateTime, #[serde(with = "time::serde::iso8601")] - end: OffsetDateTime, - extensions: HashMap, + pub end: OffsetDateTime, + pub extensions: HashMap, } id_type!(OrgId); @@ -1580,31 +1631,17 @@ pub struct Signup { } #[derive(FromRow, PartialEq, Debug, Serialize, Deserialize)] -pub struct SignupInvite { +pub struct Invite { pub email_address: String, pub email_confirmation_code: String, } #[derive(Debug, Serialize, Deserialize)] -pub struct SignupRedemption { - pub email_address: String, - pub email_confirmation_code: String, +pub struct NewUserParams { pub github_login: String, pub invite_count: i32, } -fn fuzzy_like_string(string: &str) -> String { - let mut result = String::with_capacity(string.len() * 2 + 1); - for c in string.chars() { - if c.is_alphanumeric() { - result.push('%'); - result.push(c); - } - } - result.push('%'); - result -} - fn random_invite_code() -> String { nanoid::nanoid!(16) } @@ -1614,11 +1651,14 @@ fn random_email_confirmation_code() -> String { } #[cfg(test)] -pub mod tests { +pub use test::*; + +#[cfg(test)] +mod test { use super::*; use anyhow::anyhow; use collections::BTreeMap; - use gpui::executor::{Background, Deterministic}; + use gpui::executor::Background; use lazy_static::lazy_static; use parking_lot::Mutex; use rand::prelude::*; @@ -1629,1077 +1669,6 @@ pub mod tests { use std::{path::Path, sync::Arc}; use util::post_inc; - #[tokio::test(flavor = "multi_thread")] - async fn test_get_users_by_ids() { - for test_db in [ - TestDb::postgres().await, - TestDb::fake(build_background_executor()), - ] { - let db = test_db.db(); - - let user = db.create_user("user", None, false).await.unwrap(); - let friend1 = db.create_user("friend-1", None, false).await.unwrap(); - let friend2 = db.create_user("friend-2", None, false).await.unwrap(); - let friend3 = db.create_user("friend-3", None, false).await.unwrap(); - - assert_eq!( - db.get_users_by_ids(vec![user, friend1, friend2, friend3]) - .await - .unwrap(), - vec![ - User { - id: user, - github_login: "user".to_string(), - admin: false, - ..Default::default() - }, - User { - id: friend1, - github_login: "friend-1".to_string(), - admin: false, - ..Default::default() - }, - User { - id: friend2, - github_login: "friend-2".to_string(), - admin: false, - ..Default::default() - }, - User { - id: friend3, - github_login: "friend-3".to_string(), - admin: false, - ..Default::default() - } - ] - ); - } - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_create_users() { - let db = TestDb::postgres().await; - let db = db.db(); - - // Create the first batch of users, ensuring invite counts are assigned - // correctly and the respective invite codes are unique. - let user_ids_batch_1 = db - .create_users(vec![ - ("user1".to_string(), "hi@user1.com".to_string(), 5), - ("user2".to_string(), "hi@user2.com".to_string(), 4), - ("user3".to_string(), "hi@user3.com".to_string(), 3), - ]) - .await - .unwrap(); - assert_eq!(user_ids_batch_1.len(), 3); - - let users = db.get_users_by_ids(user_ids_batch_1.clone()).await.unwrap(); - assert_eq!(users.len(), 3); - assert_eq!(users[0].github_login, "user1"); - assert_eq!(users[0].email_address.as_deref(), Some("hi@user1.com")); - assert_eq!(users[0].invite_count, 5); - assert_eq!(users[1].github_login, "user2"); - assert_eq!(users[1].email_address.as_deref(), Some("hi@user2.com")); - assert_eq!(users[1].invite_count, 4); - assert_eq!(users[2].github_login, "user3"); - assert_eq!(users[2].email_address.as_deref(), Some("hi@user3.com")); - assert_eq!(users[2].invite_count, 3); - - let invite_code_1 = users[0].invite_code.clone().unwrap(); - let invite_code_2 = users[1].invite_code.clone().unwrap(); - let invite_code_3 = users[2].invite_code.clone().unwrap(); - assert_ne!(invite_code_1, invite_code_2); - assert_ne!(invite_code_1, invite_code_3); - assert_ne!(invite_code_2, invite_code_3); - - // Create the second batch of users and include a user that is already in the database, ensuring - // the invite count for the existing user is updated without changing their invite code. - let user_ids_batch_2 = db - .create_users(vec![ - ("user2".to_string(), "hi@user2.com".to_string(), 10), - ("user4".to_string(), "hi@user4.com".to_string(), 2), - ]) - .await - .unwrap(); - assert_eq!(user_ids_batch_2.len(), 2); - assert_eq!(user_ids_batch_2[0], user_ids_batch_1[1]); - - let users = db.get_users_by_ids(user_ids_batch_2).await.unwrap(); - assert_eq!(users.len(), 2); - assert_eq!(users[0].github_login, "user2"); - assert_eq!(users[0].email_address.as_deref(), Some("hi@user2.com")); - assert_eq!(users[0].invite_count, 10); - assert_eq!(users[0].invite_code, Some(invite_code_2.clone())); - assert_eq!(users[1].github_login, "user4"); - assert_eq!(users[1].email_address.as_deref(), Some("hi@user4.com")); - assert_eq!(users[1].invite_count, 2); - - let invite_code_4 = users[1].invite_code.clone().unwrap(); - assert_ne!(invite_code_4, invite_code_1); - assert_ne!(invite_code_4, invite_code_2); - assert_ne!(invite_code_4, invite_code_3); - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_worktree_extensions() { - let test_db = TestDb::postgres().await; - let db = test_db.db(); - - let user = db.create_user("user_1", None, false).await.unwrap(); - let project = db.register_project(user).await.unwrap(); - - db.update_worktree_extensions(project, 100, Default::default()) - .await - .unwrap(); - db.update_worktree_extensions( - project, - 100, - [("rs".to_string(), 5), ("md".to_string(), 3)] - .into_iter() - .collect(), - ) - .await - .unwrap(); - db.update_worktree_extensions( - project, - 100, - [("rs".to_string(), 6), ("md".to_string(), 5)] - .into_iter() - .collect(), - ) - .await - .unwrap(); - db.update_worktree_extensions( - project, - 101, - [("ts".to_string(), 2), ("md".to_string(), 1)] - .into_iter() - .collect(), - ) - .await - .unwrap(); - - assert_eq!( - db.get_project_extensions(project).await.unwrap(), - [ - ( - 100, - [("rs".into(), 6), ("md".into(), 5),] - .into_iter() - .collect::>() - ), - ( - 101, - [("ts".into(), 2), ("md".into(), 1),] - .into_iter() - .collect::>() - ) - ] - .into_iter() - .collect() - ); - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_user_activity() { - let test_db = TestDb::postgres().await; - let db = test_db.db(); - - let user_1 = db.create_user("user_1", None, false).await.unwrap(); - let user_2 = db.create_user("user_2", None, false).await.unwrap(); - let user_3 = db.create_user("user_3", None, false).await.unwrap(); - let project_1 = db.register_project(user_1).await.unwrap(); - db.update_worktree_extensions( - project_1, - 1, - HashMap::from_iter([("rs".into(), 5), ("md".into(), 7)]), - ) - .await - .unwrap(); - let project_2 = db.register_project(user_2).await.unwrap(); - let t0 = OffsetDateTime::now_utc() - Duration::from_secs(60 * 60); - - // User 2 opens a project - let t1 = t0 + Duration::from_secs(10); - db.record_user_activity(t0..t1, &[(user_2, project_2)]) - .await - .unwrap(); - - let t2 = t1 + Duration::from_secs(10); - db.record_user_activity(t1..t2, &[(user_2, project_2)]) - .await - .unwrap(); - - // User 1 joins the project - let t3 = t2 + Duration::from_secs(10); - db.record_user_activity(t2..t3, &[(user_2, project_2), (user_1, project_2)]) - .await - .unwrap(); - - // User 1 opens another project - let t4 = t3 + Duration::from_secs(10); - db.record_user_activity( - t3..t4, - &[ - (user_2, project_2), - (user_1, project_2), - (user_1, project_1), - ], - ) - .await - .unwrap(); - - // User 3 joins that project - let t5 = t4 + Duration::from_secs(10); - db.record_user_activity( - t4..t5, - &[ - (user_2, project_2), - (user_1, project_2), - (user_1, project_1), - (user_3, project_1), - ], - ) - .await - .unwrap(); - - // User 2 leaves - let t6 = t5 + Duration::from_secs(5); - db.record_user_activity(t5..t6, &[(user_1, project_1), (user_3, project_1)]) - .await - .unwrap(); - - let t7 = t6 + Duration::from_secs(60); - let t8 = t7 + Duration::from_secs(10); - db.record_user_activity(t7..t8, &[(user_1, project_1)]) - .await - .unwrap(); - - assert_eq!( - db.get_top_users_activity_summary(t0..t6, 10).await.unwrap(), - &[ - UserActivitySummary { - id: user_1, - github_login: "user_1".to_string(), - project_activity: vec![ - ProjectActivitySummary { - id: project_1, - duration: Duration::from_secs(25), - max_collaborators: 2 - }, - ProjectActivitySummary { - id: project_2, - duration: Duration::from_secs(30), - max_collaborators: 2 - } - ] - }, - UserActivitySummary { - id: user_2, - github_login: "user_2".to_string(), - project_activity: vec![ProjectActivitySummary { - id: project_2, - duration: Duration::from_secs(50), - max_collaborators: 2 - }] - }, - UserActivitySummary { - id: user_3, - github_login: "user_3".to_string(), - project_activity: vec![ProjectActivitySummary { - id: project_1, - duration: Duration::from_secs(15), - max_collaborators: 2 - }] - }, - ] - ); - - assert_eq!( - db.get_active_user_count(t0..t6, Duration::from_secs(56), false) - .await - .unwrap(), - 0 - ); - assert_eq!( - db.get_active_user_count(t0..t6, Duration::from_secs(56), true) - .await - .unwrap(), - 0 - ); - assert_eq!( - db.get_active_user_count(t0..t6, Duration::from_secs(54), false) - .await - .unwrap(), - 1 - ); - assert_eq!( - db.get_active_user_count(t0..t6, Duration::from_secs(54), true) - .await - .unwrap(), - 1 - ); - assert_eq!( - db.get_active_user_count(t0..t6, Duration::from_secs(30), false) - .await - .unwrap(), - 2 - ); - assert_eq!( - db.get_active_user_count(t0..t6, Duration::from_secs(30), true) - .await - .unwrap(), - 2 - ); - assert_eq!( - db.get_active_user_count(t0..t6, Duration::from_secs(10), false) - .await - .unwrap(), - 3 - ); - assert_eq!( - db.get_active_user_count(t0..t6, Duration::from_secs(10), true) - .await - .unwrap(), - 3 - ); - assert_eq!( - db.get_active_user_count(t0..t1, Duration::from_secs(5), false) - .await - .unwrap(), - 1 - ); - assert_eq!( - db.get_active_user_count(t0..t1, Duration::from_secs(5), true) - .await - .unwrap(), - 0 - ); - - assert_eq!( - db.get_user_activity_timeline(t3..t6, user_1).await.unwrap(), - &[ - UserActivityPeriod { - project_id: project_1, - start: t3, - end: t6, - extensions: HashMap::from_iter([("rs".to_string(), 5), ("md".to_string(), 7)]), - }, - UserActivityPeriod { - project_id: project_2, - start: t3, - end: t5, - extensions: Default::default(), - }, - ] - ); - assert_eq!( - db.get_user_activity_timeline(t0..t8, user_1).await.unwrap(), - &[ - UserActivityPeriod { - project_id: project_2, - start: t2, - end: t5, - extensions: Default::default(), - }, - UserActivityPeriod { - project_id: project_1, - start: t3, - end: t6, - extensions: HashMap::from_iter([("rs".to_string(), 5), ("md".to_string(), 7)]), - }, - UserActivityPeriod { - project_id: project_1, - start: t7, - end: t8, - extensions: HashMap::from_iter([("rs".to_string(), 5), ("md".to_string(), 7)]), - }, - ] - ); - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_recent_channel_messages() { - for test_db in [ - TestDb::postgres().await, - TestDb::fake(build_background_executor()), - ] { - let db = test_db.db(); - let user = db.create_user("user", None, false).await.unwrap(); - let org = db.create_org("org", "org").await.unwrap(); - let channel = db.create_org_channel(org, "channel").await.unwrap(); - for i in 0..10 { - db.create_channel_message( - channel, - user, - &i.to_string(), - OffsetDateTime::now_utc(), - i, - ) - .await - .unwrap(); - } - - let messages = db.get_channel_messages(channel, 5, None).await.unwrap(); - assert_eq!( - messages.iter().map(|m| &m.body).collect::>(), - ["5", "6", "7", "8", "9"] - ); - - let prev_messages = db - .get_channel_messages(channel, 4, Some(messages[0].id)) - .await - .unwrap(); - assert_eq!( - prev_messages.iter().map(|m| &m.body).collect::>(), - ["1", "2", "3", "4"] - ); - } - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_channel_message_nonces() { - for test_db in [ - TestDb::postgres().await, - TestDb::fake(build_background_executor()), - ] { - let db = test_db.db(); - let user = db.create_user("user", None, false).await.unwrap(); - let org = db.create_org("org", "org").await.unwrap(); - let channel = db.create_org_channel(org, "channel").await.unwrap(); - - let msg1_id = db - .create_channel_message(channel, user, "1", OffsetDateTime::now_utc(), 1) - .await - .unwrap(); - let msg2_id = db - .create_channel_message(channel, user, "2", OffsetDateTime::now_utc(), 2) - .await - .unwrap(); - let msg3_id = db - .create_channel_message(channel, user, "3", OffsetDateTime::now_utc(), 1) - .await - .unwrap(); - let msg4_id = db - .create_channel_message(channel, user, "4", OffsetDateTime::now_utc(), 2) - .await - .unwrap(); - - assert_ne!(msg1_id, msg2_id); - assert_eq!(msg1_id, msg3_id); - assert_eq!(msg2_id, msg4_id); - } - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_create_access_tokens() { - let test_db = TestDb::postgres().await; - let db = test_db.db(); - let user = db.create_user("the-user", None, false).await.unwrap(); - - db.create_access_token_hash(user, "h1", 3).await.unwrap(); - db.create_access_token_hash(user, "h2", 3).await.unwrap(); - assert_eq!( - db.get_access_token_hashes(user).await.unwrap(), - &["h2".to_string(), "h1".to_string()] - ); - - db.create_access_token_hash(user, "h3", 3).await.unwrap(); - assert_eq!( - db.get_access_token_hashes(user).await.unwrap(), - &["h3".to_string(), "h2".to_string(), "h1".to_string(),] - ); - - db.create_access_token_hash(user, "h4", 3).await.unwrap(); - assert_eq!( - db.get_access_token_hashes(user).await.unwrap(), - &["h4".to_string(), "h3".to_string(), "h2".to_string(),] - ); - - db.create_access_token_hash(user, "h5", 3).await.unwrap(); - assert_eq!( - db.get_access_token_hashes(user).await.unwrap(), - &["h5".to_string(), "h4".to_string(), "h3".to_string()] - ); - } - - #[test] - fn test_fuzzy_like_string() { - assert_eq!(fuzzy_like_string("abcd"), "%a%b%c%d%"); - assert_eq!(fuzzy_like_string("x y"), "%x%y%"); - assert_eq!(fuzzy_like_string(" z "), "%z%"); - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_fuzzy_search_users() { - let test_db = TestDb::postgres().await; - let db = test_db.db(); - for github_login in [ - "California", - "colorado", - "oregon", - "washington", - "florida", - "delaware", - "rhode-island", - ] { - db.create_user(github_login, None, false).await.unwrap(); - } - - assert_eq!( - fuzzy_search_user_names(db, "clr").await, - &["colorado", "California"] - ); - assert_eq!( - fuzzy_search_user_names(db, "ro").await, - &["rhode-island", "colorado", "oregon"], - ); - - async fn fuzzy_search_user_names(db: &Arc, query: &str) -> Vec { - db.fuzzy_search_users(query, 10) - .await - .unwrap() - .into_iter() - .map(|user| user.github_login) - .collect::>() - } - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_add_contacts() { - for test_db in [ - TestDb::postgres().await, - TestDb::fake(build_background_executor()), - ] { - let db = test_db.db(); - - let user_1 = db.create_user("user1", None, false).await.unwrap(); - let user_2 = db.create_user("user2", None, false).await.unwrap(); - let user_3 = db.create_user("user3", None, false).await.unwrap(); - - // User starts with no contacts - assert_eq!( - db.get_contacts(user_1).await.unwrap(), - vec![Contact::Accepted { - user_id: user_1, - should_notify: false - }], - ); - - // User requests a contact. Both users see the pending request. - db.send_contact_request(user_1, user_2).await.unwrap(); - assert!(!db.has_contact(user_1, user_2).await.unwrap()); - assert!(!db.has_contact(user_2, user_1).await.unwrap()); - assert_eq!( - db.get_contacts(user_1).await.unwrap(), - &[ - Contact::Accepted { - user_id: user_1, - should_notify: false - }, - Contact::Outgoing { user_id: user_2 } - ], - ); - assert_eq!( - db.get_contacts(user_2).await.unwrap(), - &[ - Contact::Incoming { - user_id: user_1, - should_notify: true - }, - Contact::Accepted { - user_id: user_2, - should_notify: false - }, - ] - ); - - // User 2 dismisses the contact request notification without accepting or rejecting. - // We shouldn't notify them again. - db.dismiss_contact_notification(user_1, user_2) - .await - .unwrap_err(); - db.dismiss_contact_notification(user_2, user_1) - .await - .unwrap(); - assert_eq!( - db.get_contacts(user_2).await.unwrap(), - &[ - Contact::Incoming { - user_id: user_1, - should_notify: false - }, - Contact::Accepted { - user_id: user_2, - should_notify: false - }, - ] - ); - - // User can't accept their own contact request - db.respond_to_contact_request(user_1, user_2, true) - .await - .unwrap_err(); - - // User accepts a contact request. Both users see the contact. - db.respond_to_contact_request(user_2, user_1, true) - .await - .unwrap(); - assert_eq!( - db.get_contacts(user_1).await.unwrap(), - &[ - Contact::Accepted { - user_id: user_1, - should_notify: false - }, - Contact::Accepted { - user_id: user_2, - should_notify: true - } - ], - ); - assert!(db.has_contact(user_1, user_2).await.unwrap()); - assert!(db.has_contact(user_2, user_1).await.unwrap()); - assert_eq!( - db.get_contacts(user_2).await.unwrap(), - &[ - Contact::Accepted { - user_id: user_1, - should_notify: false, - }, - Contact::Accepted { - user_id: user_2, - should_notify: false, - }, - ] - ); - - // Users cannot re-request existing contacts. - db.send_contact_request(user_1, user_2).await.unwrap_err(); - db.send_contact_request(user_2, user_1).await.unwrap_err(); - - // Users can't dismiss notifications of them accepting other users' requests. - db.dismiss_contact_notification(user_2, user_1) - .await - .unwrap_err(); - assert_eq!( - db.get_contacts(user_1).await.unwrap(), - &[ - Contact::Accepted { - user_id: user_1, - should_notify: false - }, - Contact::Accepted { - user_id: user_2, - should_notify: true, - }, - ] - ); - - // Users can dismiss notifications of other users accepting their requests. - db.dismiss_contact_notification(user_1, user_2) - .await - .unwrap(); - assert_eq!( - db.get_contacts(user_1).await.unwrap(), - &[ - Contact::Accepted { - user_id: user_1, - should_notify: false - }, - Contact::Accepted { - user_id: user_2, - should_notify: false, - }, - ] - ); - - // Users send each other concurrent contact requests and - // see that they are immediately accepted. - db.send_contact_request(user_1, user_3).await.unwrap(); - db.send_contact_request(user_3, user_1).await.unwrap(); - assert_eq!( - db.get_contacts(user_1).await.unwrap(), - &[ - Contact::Accepted { - user_id: user_1, - should_notify: false - }, - Contact::Accepted { - user_id: user_2, - should_notify: false, - }, - Contact::Accepted { - user_id: user_3, - should_notify: false - }, - ] - ); - assert_eq!( - db.get_contacts(user_3).await.unwrap(), - &[ - Contact::Accepted { - user_id: user_1, - should_notify: false - }, - Contact::Accepted { - user_id: user_3, - should_notify: false - } - ], - ); - - // User declines a contact request. Both users see that it is gone. - db.send_contact_request(user_2, user_3).await.unwrap(); - db.respond_to_contact_request(user_3, user_2, false) - .await - .unwrap(); - assert!(!db.has_contact(user_2, user_3).await.unwrap()); - assert!(!db.has_contact(user_3, user_2).await.unwrap()); - assert_eq!( - db.get_contacts(user_2).await.unwrap(), - &[ - Contact::Accepted { - user_id: user_1, - should_notify: false - }, - Contact::Accepted { - user_id: user_2, - should_notify: false - } - ] - ); - assert_eq!( - db.get_contacts(user_3).await.unwrap(), - &[ - Contact::Accepted { - user_id: user_1, - should_notify: false - }, - Contact::Accepted { - user_id: user_3, - should_notify: false - } - ], - ); - } - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_invite_codes() { - let postgres = TestDb::postgres().await; - let db = postgres.db(); - let user1 = db.create_user("user-1", None, false).await.unwrap(); - - // Initially, user 1 has no invite code - assert_eq!(db.get_invite_code_for_user(user1).await.unwrap(), None); - - // Setting invite count to 0 when no code is assigned does not assign a new code - db.set_invite_count(user1, 0).await.unwrap(); - assert!(db.get_invite_code_for_user(user1).await.unwrap().is_none()); - - // User 1 creates an invite code that can be used twice. - db.set_invite_count(user1, 2).await.unwrap(); - let (invite_code, invite_count) = - db.get_invite_code_for_user(user1).await.unwrap().unwrap(); - assert_eq!(invite_count, 2); - - // User 2 redeems the invite code and becomes a contact of user 1. - let user2 = db - .redeem_invite_code(&invite_code, "user-2", None) - .await - .unwrap(); - let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); - assert_eq!(invite_count, 1); - assert_eq!( - db.get_contacts(user1).await.unwrap(), - [ - Contact::Accepted { - user_id: user1, - should_notify: false - }, - Contact::Accepted { - user_id: user2, - should_notify: true - } - ] - ); - assert_eq!( - db.get_contacts(user2).await.unwrap(), - [ - Contact::Accepted { - user_id: user1, - should_notify: false - }, - Contact::Accepted { - user_id: user2, - should_notify: false - } - ] - ); - - // User 3 redeems the invite code and becomes a contact of user 1. - let user3 = db - .redeem_invite_code(&invite_code, "user-3", None) - .await - .unwrap(); - let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); - assert_eq!(invite_count, 0); - assert_eq!( - db.get_contacts(user1).await.unwrap(), - [ - Contact::Accepted { - user_id: user1, - should_notify: false - }, - Contact::Accepted { - user_id: user2, - should_notify: true - }, - Contact::Accepted { - user_id: user3, - should_notify: true - } - ] - ); - assert_eq!( - db.get_contacts(user3).await.unwrap(), - [ - Contact::Accepted { - user_id: user1, - should_notify: false - }, - Contact::Accepted { - user_id: user3, - should_notify: false - }, - ] - ); - - // Trying to reedem the code for the third time results in an error. - db.redeem_invite_code(&invite_code, "user-4", None) - .await - .unwrap_err(); - - // Invite count can be updated after the code has been created. - db.set_invite_count(user1, 2).await.unwrap(); - let (latest_code, invite_count) = - db.get_invite_code_for_user(user1).await.unwrap().unwrap(); - assert_eq!(latest_code, invite_code); // Invite code doesn't change when we increment above 0 - assert_eq!(invite_count, 2); - - // User 4 can now redeem the invite code and becomes a contact of user 1. - let user4 = db - .redeem_invite_code(&invite_code, "user-4", None) - .await - .unwrap(); - let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); - assert_eq!(invite_count, 1); - assert_eq!( - db.get_contacts(user1).await.unwrap(), - [ - Contact::Accepted { - user_id: user1, - should_notify: false - }, - Contact::Accepted { - user_id: user2, - should_notify: true - }, - Contact::Accepted { - user_id: user3, - should_notify: true - }, - Contact::Accepted { - user_id: user4, - should_notify: true - } - ] - ); - assert_eq!( - db.get_contacts(user4).await.unwrap(), - [ - Contact::Accepted { - user_id: user1, - should_notify: false - }, - Contact::Accepted { - user_id: user4, - should_notify: false - }, - ] - ); - - // An existing user cannot redeem invite codes. - db.redeem_invite_code(&invite_code, "user-2", None) - .await - .unwrap_err(); - let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); - assert_eq!(invite_count, 1); - - // Ensure invited users get invite codes too. - assert_eq!( - db.get_invite_code_for_user(user2).await.unwrap().unwrap().1, - 5 - ); - assert_eq!( - db.get_invite_code_for_user(user3).await.unwrap().unwrap().1, - 5 - ); - assert_eq!( - db.get_invite_code_for_user(user4).await.unwrap().unwrap().1, - 5 - ); - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_signups() { - let postgres = TestDb::postgres().await; - let db = postgres.db(); - - // people sign up on the waitlist - for i in 0..8 { - db.create_signup(Signup { - email_address: format!("person-{i}@example.com"), - platform_mac: true, - platform_linux: true, - platform_windows: false, - editor_features: vec!["speed".into()], - programming_languages: vec!["rust".into(), "c".into()], - }) - .await - .unwrap(); - } - - // retrieve the next batch of signup emails to send - let signups_batch1 = db.get_signup_invites(3).await.unwrap(); - let addresses = signups_batch1 - .iter() - .map(|s| &s.email_address) - .collect::>(); - assert_eq!( - addresses, - &[ - "person-0@example.com", - "person-1@example.com", - "person-2@example.com" - ] - ); - assert_ne!( - signups_batch1[0].email_confirmation_code, - signups_batch1[1].email_confirmation_code - ); - - // the waitlist isn't updated until we record that the emails - // were successfully sent. - let signups_batch = db.get_signup_invites(3).await.unwrap(); - assert_eq!(signups_batch, signups_batch1); - - // once the emails go out, we can retrieve the next batch - // of signups. - db.record_signup_invites_sent(&signups_batch1) - .await - .unwrap(); - let signups_batch2 = db.get_signup_invites(3).await.unwrap(); - let addresses = signups_batch2 - .iter() - .map(|s| &s.email_address) - .collect::>(); - assert_eq!( - addresses, - &[ - "person-3@example.com", - "person-4@example.com", - "person-5@example.com" - ] - ); - - // user completes the signup process by providing their - // github account. - let user_id = db - .redeem_signup(SignupRedemption { - email_address: signups_batch1[0].email_address.clone(), - email_confirmation_code: signups_batch1[0].email_confirmation_code.clone(), - github_login: "person-0".into(), - invite_count: 5, - }) - .await - .unwrap(); - let user = db.get_user_by_id(user_id).await.unwrap().unwrap(); - assert_eq!(user.github_login, "person-0"); - assert_eq!(user.email_address.as_deref(), Some("person-0@example.com")); - assert_eq!(user.invite_count, 5); - - // cannot redeem the same signup again. - db.redeem_signup(SignupRedemption { - email_address: signups_batch1[0].email_address.clone(), - email_confirmation_code: signups_batch1[0].email_confirmation_code.clone(), - github_login: "some-other-github_account".into(), - invite_count: 5, - }) - .await - .unwrap_err(); - - // cannot redeem a signup with the wrong confirmation code. - db.redeem_signup(SignupRedemption { - email_address: signups_batch1[1].email_address.clone(), - email_confirmation_code: "the-wrong-code".to_string(), - github_login: "person-1".into(), - invite_count: 5, - }) - .await - .unwrap_err(); - } - - pub struct TestDb { - pub db: Option>, - pub url: String, - } - - impl TestDb { - #[allow(clippy::await_holding_lock)] - pub async fn postgres() -> Self { - lazy_static! { - static ref LOCK: Mutex<()> = Mutex::new(()); - } - - let _guard = LOCK.lock(); - let mut rng = StdRng::from_entropy(); - let name = format!("zed-test-{}", rng.gen::()); - let url = format!("postgres://postgres@localhost/{}", name); - let migrations_path = Path::new(concat!(env!("CARGO_MANIFEST_DIR"), "/migrations")); - Postgres::create_database(&url) - .await - .expect("failed to create test db"); - let db = PostgresDb::new(&url, 5).await.unwrap(); - let migrator = Migrator::new(migrations_path).await.unwrap(); - migrator.run(&db.pool).await.unwrap(); - Self { - db: Some(Arc::new(db)), - url, - } - } - - pub fn fake(background: Arc) -> Self { - Self { - db: Some(Arc::new(FakeDb::new(background))), - url: Default::default(), - } - } - - pub fn db(&self) -> &Arc { - self.db.as_ref().unwrap() - } - } - - impl Drop for TestDb { - fn drop(&mut self) { - if let Some(db) = self.db.take() { - futures::executor::block_on(db.teardown(&self.url)); - } - } - } - pub struct FakeDb { background: Arc, pub users: Mutex>, @@ -2753,7 +1722,7 @@ pub mod tests { async fn create_user( &self, github_login: &str, - email_address: Option<&str>, + email_address: &str, admin: bool, ) -> Result { self.background.simulate_random_delay().await; @@ -2771,7 +1740,7 @@ pub mod tests { User { id: user_id, github_login: github_login.to_string(), - email_address: email_address.map(str::to_string), + email_address: Some(email_address.to_string()), admin, invite_code: None, invite_count: 0, @@ -2843,24 +1812,25 @@ pub mod tests { unimplemented!() } - async fn get_signup_invites(&self, _count: usize) -> Result> { + async fn get_unsent_invites(&self, _count: usize) -> Result> { unimplemented!() } - async fn record_signup_invites_sent(&self, _signups: &[SignupInvite]) -> Result<()> { + async fn record_sent_invites(&self, _invites: &[Invite]) -> Result<()> { unimplemented!() } - async fn redeem_signup( + async fn create_user_from_invite( &self, - _redemption: SignupRedemption, - ) -> Result { + _invite: &Invite, + _user: NewUserParams, + ) -> Result<(UserId, Option)> { unimplemented!() } // invite codes - async fn set_invite_count(&self, _id: UserId, _count: u32) -> Result<()> { + async fn set_invite_count_for_user(&self, _id: UserId, _count: u32) -> Result<()> { unimplemented!() } @@ -2873,12 +1843,11 @@ pub mod tests { unimplemented!() } - async fn redeem_invite_code( + async fn create_invite_from_code( &self, _code: &str, - _login: &str, - _email_address: Option<&str>, - ) -> Result { + _email_address: &str, + ) -> Result { unimplemented!() } @@ -3316,7 +2285,52 @@ pub mod tests { } } - fn build_background_executor() -> Arc { - Deterministic::new(0).build_background() + pub struct TestDb { + pub db: Option>, + pub url: String, + } + + impl TestDb { + #[allow(clippy::await_holding_lock)] + pub async fn postgres() -> Self { + lazy_static! { + static ref LOCK: Mutex<()> = Mutex::new(()); + } + + let _guard = LOCK.lock(); + let mut rng = StdRng::from_entropy(); + let name = format!("zed-test-{}", rng.gen::()); + let url = format!("postgres://postgres@localhost/{}", name); + let migrations_path = Path::new(concat!(env!("CARGO_MANIFEST_DIR"), "/migrations")); + Postgres::create_database(&url) + .await + .expect("failed to create test db"); + let db = PostgresDb::new(&url, 5).await.unwrap(); + let migrator = Migrator::new(migrations_path).await.unwrap(); + migrator.run(&db.pool).await.unwrap(); + Self { + db: Some(Arc::new(db)), + url, + } + } + + pub fn fake(background: Arc) -> Self { + Self { + db: Some(Arc::new(FakeDb::new(background))), + url: Default::default(), + } + } + + pub fn db(&self) -> &Arc { + self.db.as_ref().unwrap() + } + } + + impl Drop for TestDb { + fn drop(&mut self) { + if let Some(db) = self.db.take() { + futures::executor::block_on(db.teardown(&self.url)); + } + } } } diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs new file mode 100644 index 0000000000..aa9a0b6995 --- /dev/null +++ b/crates/collab/src/db_tests.rs @@ -0,0 +1,1071 @@ +use super::db::*; +use collections::HashMap; +use gpui::executor::{Background, Deterministic}; +use std::{sync::Arc, time::Duration}; +use time::OffsetDateTime; + +#[tokio::test(flavor = "multi_thread")] +async fn test_get_users_by_ids() { + for test_db in [ + TestDb::postgres().await, + TestDb::fake(build_background_executor()), + ] { + let db = test_db.db(); + + let user1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); + let user2 = db.create_user("u2", "u2@example.com", false).await.unwrap(); + let user3 = db.create_user("u3", "u3@example.com", false).await.unwrap(); + let user4 = db.create_user("u4", "u4@example.com", false).await.unwrap(); + + assert_eq!( + db.get_users_by_ids(vec![user1, user2, user3, user4]) + .await + .unwrap(), + vec![ + User { + id: user1, + github_login: "u1".to_string(), + email_address: Some("u1@example.com".to_string()), + admin: false, + ..Default::default() + }, + User { + id: user2, + github_login: "u2".to_string(), + email_address: Some("u2@example.com".to_string()), + admin: false, + ..Default::default() + }, + User { + id: user3, + github_login: "u3".to_string(), + email_address: Some("u3@example.com".to_string()), + admin: false, + ..Default::default() + }, + User { + id: user4, + github_login: "u4".to_string(), + email_address: Some("u4@example.com".to_string()), + admin: false, + ..Default::default() + } + ] + ); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_create_users() { + let db = TestDb::postgres().await; + let db = db.db(); + + // Create the first batch of users, ensuring invite counts are assigned + // correctly and the respective invite codes are unique. + let user_ids_batch_1 = db + .create_users(vec![ + ("user1".to_string(), "hi@user1.com".to_string(), 5), + ("user2".to_string(), "hi@user2.com".to_string(), 4), + ("user3".to_string(), "hi@user3.com".to_string(), 3), + ]) + .await + .unwrap(); + assert_eq!(user_ids_batch_1.len(), 3); + + let users = db.get_users_by_ids(user_ids_batch_1.clone()).await.unwrap(); + assert_eq!(users.len(), 3); + assert_eq!(users[0].github_login, "user1"); + assert_eq!(users[0].email_address.as_deref(), Some("hi@user1.com")); + assert_eq!(users[0].invite_count, 5); + assert_eq!(users[1].github_login, "user2"); + assert_eq!(users[1].email_address.as_deref(), Some("hi@user2.com")); + assert_eq!(users[1].invite_count, 4); + assert_eq!(users[2].github_login, "user3"); + assert_eq!(users[2].email_address.as_deref(), Some("hi@user3.com")); + assert_eq!(users[2].invite_count, 3); + + let invite_code_1 = users[0].invite_code.clone().unwrap(); + let invite_code_2 = users[1].invite_code.clone().unwrap(); + let invite_code_3 = users[2].invite_code.clone().unwrap(); + assert_ne!(invite_code_1, invite_code_2); + assert_ne!(invite_code_1, invite_code_3); + assert_ne!(invite_code_2, invite_code_3); + + // Create the second batch of users and include a user that is already in the database, ensuring + // the invite count for the existing user is updated without changing their invite code. + let user_ids_batch_2 = db + .create_users(vec![ + ("user2".to_string(), "hi@user2.com".to_string(), 10), + ("user4".to_string(), "hi@user4.com".to_string(), 2), + ]) + .await + .unwrap(); + assert_eq!(user_ids_batch_2.len(), 2); + assert_eq!(user_ids_batch_2[0], user_ids_batch_1[1]); + + let users = db.get_users_by_ids(user_ids_batch_2).await.unwrap(); + assert_eq!(users.len(), 2); + assert_eq!(users[0].github_login, "user2"); + assert_eq!(users[0].email_address.as_deref(), Some("hi@user2.com")); + assert_eq!(users[0].invite_count, 10); + assert_eq!(users[0].invite_code, Some(invite_code_2.clone())); + assert_eq!(users[1].github_login, "user4"); + assert_eq!(users[1].email_address.as_deref(), Some("hi@user4.com")); + assert_eq!(users[1].invite_count, 2); + + let invite_code_4 = users[1].invite_code.clone().unwrap(); + assert_ne!(invite_code_4, invite_code_1); + assert_ne!(invite_code_4, invite_code_2); + assert_ne!(invite_code_4, invite_code_3); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_worktree_extensions() { + let test_db = TestDb::postgres().await; + let db = test_db.db(); + + let user = db.create_user("u1", "u1@example.com", false).await.unwrap(); + let project = db.register_project(user).await.unwrap(); + + db.update_worktree_extensions(project, 100, Default::default()) + .await + .unwrap(); + db.update_worktree_extensions( + project, + 100, + [("rs".to_string(), 5), ("md".to_string(), 3)] + .into_iter() + .collect(), + ) + .await + .unwrap(); + db.update_worktree_extensions( + project, + 100, + [("rs".to_string(), 6), ("md".to_string(), 5)] + .into_iter() + .collect(), + ) + .await + .unwrap(); + db.update_worktree_extensions( + project, + 101, + [("ts".to_string(), 2), ("md".to_string(), 1)] + .into_iter() + .collect(), + ) + .await + .unwrap(); + + assert_eq!( + db.get_project_extensions(project).await.unwrap(), + [ + ( + 100, + [("rs".into(), 6), ("md".into(), 5),] + .into_iter() + .collect::>() + ), + ( + 101, + [("ts".into(), 2), ("md".into(), 1),] + .into_iter() + .collect::>() + ) + ] + .into_iter() + .collect() + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_user_activity() { + let test_db = TestDb::postgres().await; + let db = test_db.db(); + + let user_1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); + let user_2 = db.create_user("u2", "u2@example.com", false).await.unwrap(); + let user_3 = db.create_user("u3", "u3@example.com", false).await.unwrap(); + let project_1 = db.register_project(user_1).await.unwrap(); + db.update_worktree_extensions( + project_1, + 1, + HashMap::from_iter([("rs".into(), 5), ("md".into(), 7)]), + ) + .await + .unwrap(); + let project_2 = db.register_project(user_2).await.unwrap(); + let t0 = OffsetDateTime::now_utc() - Duration::from_secs(60 * 60); + + // User 2 opens a project + let t1 = t0 + Duration::from_secs(10); + db.record_user_activity(t0..t1, &[(user_2, project_2)]) + .await + .unwrap(); + + let t2 = t1 + Duration::from_secs(10); + db.record_user_activity(t1..t2, &[(user_2, project_2)]) + .await + .unwrap(); + + // User 1 joins the project + let t3 = t2 + Duration::from_secs(10); + db.record_user_activity(t2..t3, &[(user_2, project_2), (user_1, project_2)]) + .await + .unwrap(); + + // User 1 opens another project + let t4 = t3 + Duration::from_secs(10); + db.record_user_activity( + t3..t4, + &[ + (user_2, project_2), + (user_1, project_2), + (user_1, project_1), + ], + ) + .await + .unwrap(); + + // User 3 joins that project + let t5 = t4 + Duration::from_secs(10); + db.record_user_activity( + t4..t5, + &[ + (user_2, project_2), + (user_1, project_2), + (user_1, project_1), + (user_3, project_1), + ], + ) + .await + .unwrap(); + + // User 2 leaves + let t6 = t5 + Duration::from_secs(5); + db.record_user_activity(t5..t6, &[(user_1, project_1), (user_3, project_1)]) + .await + .unwrap(); + + let t7 = t6 + Duration::from_secs(60); + let t8 = t7 + Duration::from_secs(10); + db.record_user_activity(t7..t8, &[(user_1, project_1)]) + .await + .unwrap(); + + assert_eq!( + db.get_top_users_activity_summary(t0..t6, 10).await.unwrap(), + &[ + UserActivitySummary { + id: user_1, + github_login: "u1".to_string(), + project_activity: vec![ + ProjectActivitySummary { + id: project_1, + duration: Duration::from_secs(25), + max_collaborators: 2 + }, + ProjectActivitySummary { + id: project_2, + duration: Duration::from_secs(30), + max_collaborators: 2 + } + ] + }, + UserActivitySummary { + id: user_2, + github_login: "u2".to_string(), + project_activity: vec![ProjectActivitySummary { + id: project_2, + duration: Duration::from_secs(50), + max_collaborators: 2 + }] + }, + UserActivitySummary { + id: user_3, + github_login: "u3".to_string(), + project_activity: vec![ProjectActivitySummary { + id: project_1, + duration: Duration::from_secs(15), + max_collaborators: 2 + }] + }, + ] + ); + + assert_eq!( + db.get_active_user_count(t0..t6, Duration::from_secs(56), false) + .await + .unwrap(), + 0 + ); + assert_eq!( + db.get_active_user_count(t0..t6, Duration::from_secs(56), true) + .await + .unwrap(), + 0 + ); + assert_eq!( + db.get_active_user_count(t0..t6, Duration::from_secs(54), false) + .await + .unwrap(), + 1 + ); + assert_eq!( + db.get_active_user_count(t0..t6, Duration::from_secs(54), true) + .await + .unwrap(), + 1 + ); + assert_eq!( + db.get_active_user_count(t0..t6, Duration::from_secs(30), false) + .await + .unwrap(), + 2 + ); + assert_eq!( + db.get_active_user_count(t0..t6, Duration::from_secs(30), true) + .await + .unwrap(), + 2 + ); + assert_eq!( + db.get_active_user_count(t0..t6, Duration::from_secs(10), false) + .await + .unwrap(), + 3 + ); + assert_eq!( + db.get_active_user_count(t0..t6, Duration::from_secs(10), true) + .await + .unwrap(), + 3 + ); + assert_eq!( + db.get_active_user_count(t0..t1, Duration::from_secs(5), false) + .await + .unwrap(), + 1 + ); + assert_eq!( + db.get_active_user_count(t0..t1, Duration::from_secs(5), true) + .await + .unwrap(), + 0 + ); + + assert_eq!( + db.get_user_activity_timeline(t3..t6, user_1).await.unwrap(), + &[ + UserActivityPeriod { + project_id: project_1, + start: t3, + end: t6, + extensions: HashMap::from_iter([("rs".to_string(), 5), ("md".to_string(), 7)]), + }, + UserActivityPeriod { + project_id: project_2, + start: t3, + end: t5, + extensions: Default::default(), + }, + ] + ); + assert_eq!( + db.get_user_activity_timeline(t0..t8, user_1).await.unwrap(), + &[ + UserActivityPeriod { + project_id: project_2, + start: t2, + end: t5, + extensions: Default::default(), + }, + UserActivityPeriod { + project_id: project_1, + start: t3, + end: t6, + extensions: HashMap::from_iter([("rs".to_string(), 5), ("md".to_string(), 7)]), + }, + UserActivityPeriod { + project_id: project_1, + start: t7, + end: t8, + extensions: HashMap::from_iter([("rs".to_string(), 5), ("md".to_string(), 7)]), + }, + ] + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_recent_channel_messages() { + for test_db in [ + TestDb::postgres().await, + TestDb::fake(build_background_executor()), + ] { + let db = test_db.db(); + let user = db.create_user("u", "u@example.com", false).await.unwrap(); + let org = db.create_org("org", "org").await.unwrap(); + let channel = db.create_org_channel(org, "channel").await.unwrap(); + for i in 0..10 { + db.create_channel_message(channel, user, &i.to_string(), OffsetDateTime::now_utc(), i) + .await + .unwrap(); + } + + let messages = db.get_channel_messages(channel, 5, None).await.unwrap(); + assert_eq!( + messages.iter().map(|m| &m.body).collect::>(), + ["5", "6", "7", "8", "9"] + ); + + let prev_messages = db + .get_channel_messages(channel, 4, Some(messages[0].id)) + .await + .unwrap(); + assert_eq!( + prev_messages.iter().map(|m| &m.body).collect::>(), + ["1", "2", "3", "4"] + ); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_channel_message_nonces() { + for test_db in [ + TestDb::postgres().await, + TestDb::fake(build_background_executor()), + ] { + let db = test_db.db(); + let user = db.create_user("u", "u@example.com", false).await.unwrap(); + let org = db.create_org("org", "org").await.unwrap(); + let channel = db.create_org_channel(org, "channel").await.unwrap(); + + let msg1_id = db + .create_channel_message(channel, user, "1", OffsetDateTime::now_utc(), 1) + .await + .unwrap(); + let msg2_id = db + .create_channel_message(channel, user, "2", OffsetDateTime::now_utc(), 2) + .await + .unwrap(); + let msg3_id = db + .create_channel_message(channel, user, "3", OffsetDateTime::now_utc(), 1) + .await + .unwrap(); + let msg4_id = db + .create_channel_message(channel, user, "4", OffsetDateTime::now_utc(), 2) + .await + .unwrap(); + + assert_ne!(msg1_id, msg2_id); + assert_eq!(msg1_id, msg3_id); + assert_eq!(msg2_id, msg4_id); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_create_access_tokens() { + let test_db = TestDb::postgres().await; + let db = test_db.db(); + let user = db.create_user("u1", "u1@example.com", false).await.unwrap(); + + db.create_access_token_hash(user, "h1", 3).await.unwrap(); + db.create_access_token_hash(user, "h2", 3).await.unwrap(); + assert_eq!( + db.get_access_token_hashes(user).await.unwrap(), + &["h2".to_string(), "h1".to_string()] + ); + + db.create_access_token_hash(user, "h3", 3).await.unwrap(); + assert_eq!( + db.get_access_token_hashes(user).await.unwrap(), + &["h3".to_string(), "h2".to_string(), "h1".to_string(),] + ); + + db.create_access_token_hash(user, "h4", 3).await.unwrap(); + assert_eq!( + db.get_access_token_hashes(user).await.unwrap(), + &["h4".to_string(), "h3".to_string(), "h2".to_string(),] + ); + + db.create_access_token_hash(user, "h5", 3).await.unwrap(); + assert_eq!( + db.get_access_token_hashes(user).await.unwrap(), + &["h5".to_string(), "h4".to_string(), "h3".to_string()] + ); +} + +#[test] +fn test_fuzzy_like_string() { + assert_eq!(PostgresDb::fuzzy_like_string("abcd"), "%a%b%c%d%"); + assert_eq!(PostgresDb::fuzzy_like_string("x y"), "%x%y%"); + assert_eq!(PostgresDb::fuzzy_like_string(" z "), "%z%"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_fuzzy_search_users() { + let test_db = TestDb::postgres().await; + let db = test_db.db(); + for github_login in [ + "California", + "colorado", + "oregon", + "washington", + "florida", + "delaware", + "rhode-island", + ] { + db.create_user(github_login, &format!("{github_login}@example.com"), false) + .await + .unwrap(); + } + + assert_eq!( + fuzzy_search_user_names(db, "clr").await, + &["colorado", "California"] + ); + assert_eq!( + fuzzy_search_user_names(db, "ro").await, + &["rhode-island", "colorado", "oregon"], + ); + + async fn fuzzy_search_user_names(db: &Arc, query: &str) -> Vec { + db.fuzzy_search_users(query, 10) + .await + .unwrap() + .into_iter() + .map(|user| user.github_login) + .collect::>() + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_add_contacts() { + for test_db in [ + TestDb::postgres().await, + TestDb::fake(build_background_executor()), + ] { + let db = test_db.db(); + + let user_1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); + let user_2 = db.create_user("u2", "u2@example.com", false).await.unwrap(); + let user_3 = db.create_user("u3", "u3@example.com", false).await.unwrap(); + + // User starts with no contacts + assert_eq!( + db.get_contacts(user_1).await.unwrap(), + vec![Contact::Accepted { + user_id: user_1, + should_notify: false + }], + ); + + // User requests a contact. Both users see the pending request. + db.send_contact_request(user_1, user_2).await.unwrap(); + assert!(!db.has_contact(user_1, user_2).await.unwrap()); + assert!(!db.has_contact(user_2, user_1).await.unwrap()); + assert_eq!( + db.get_contacts(user_1).await.unwrap(), + &[ + Contact::Accepted { + user_id: user_1, + should_notify: false + }, + Contact::Outgoing { user_id: user_2 } + ], + ); + assert_eq!( + db.get_contacts(user_2).await.unwrap(), + &[ + Contact::Incoming { + user_id: user_1, + should_notify: true + }, + Contact::Accepted { + user_id: user_2, + should_notify: false + }, + ] + ); + + // User 2 dismisses the contact request notification without accepting or rejecting. + // We shouldn't notify them again. + db.dismiss_contact_notification(user_1, user_2) + .await + .unwrap_err(); + db.dismiss_contact_notification(user_2, user_1) + .await + .unwrap(); + assert_eq!( + db.get_contacts(user_2).await.unwrap(), + &[ + Contact::Incoming { + user_id: user_1, + should_notify: false + }, + Contact::Accepted { + user_id: user_2, + should_notify: false + }, + ] + ); + + // User can't accept their own contact request + db.respond_to_contact_request(user_1, user_2, true) + .await + .unwrap_err(); + + // User accepts a contact request. Both users see the contact. + db.respond_to_contact_request(user_2, user_1, true) + .await + .unwrap(); + assert_eq!( + db.get_contacts(user_1).await.unwrap(), + &[ + Contact::Accepted { + user_id: user_1, + should_notify: false + }, + Contact::Accepted { + user_id: user_2, + should_notify: true + } + ], + ); + assert!(db.has_contact(user_1, user_2).await.unwrap()); + assert!(db.has_contact(user_2, user_1).await.unwrap()); + assert_eq!( + db.get_contacts(user_2).await.unwrap(), + &[ + Contact::Accepted { + user_id: user_1, + should_notify: false, + }, + Contact::Accepted { + user_id: user_2, + should_notify: false, + }, + ] + ); + + // Users cannot re-request existing contacts. + db.send_contact_request(user_1, user_2).await.unwrap_err(); + db.send_contact_request(user_2, user_1).await.unwrap_err(); + + // Users can't dismiss notifications of them accepting other users' requests. + db.dismiss_contact_notification(user_2, user_1) + .await + .unwrap_err(); + assert_eq!( + db.get_contacts(user_1).await.unwrap(), + &[ + Contact::Accepted { + user_id: user_1, + should_notify: false + }, + Contact::Accepted { + user_id: user_2, + should_notify: true, + }, + ] + ); + + // Users can dismiss notifications of other users accepting their requests. + db.dismiss_contact_notification(user_1, user_2) + .await + .unwrap(); + assert_eq!( + db.get_contacts(user_1).await.unwrap(), + &[ + Contact::Accepted { + user_id: user_1, + should_notify: false + }, + Contact::Accepted { + user_id: user_2, + should_notify: false, + }, + ] + ); + + // Users send each other concurrent contact requests and + // see that they are immediately accepted. + db.send_contact_request(user_1, user_3).await.unwrap(); + db.send_contact_request(user_3, user_1).await.unwrap(); + assert_eq!( + db.get_contacts(user_1).await.unwrap(), + &[ + Contact::Accepted { + user_id: user_1, + should_notify: false + }, + Contact::Accepted { + user_id: user_2, + should_notify: false, + }, + Contact::Accepted { + user_id: user_3, + should_notify: false + }, + ] + ); + assert_eq!( + db.get_contacts(user_3).await.unwrap(), + &[ + Contact::Accepted { + user_id: user_1, + should_notify: false + }, + Contact::Accepted { + user_id: user_3, + should_notify: false + } + ], + ); + + // User declines a contact request. Both users see that it is gone. + db.send_contact_request(user_2, user_3).await.unwrap(); + db.respond_to_contact_request(user_3, user_2, false) + .await + .unwrap(); + assert!(!db.has_contact(user_2, user_3).await.unwrap()); + assert!(!db.has_contact(user_3, user_2).await.unwrap()); + assert_eq!( + db.get_contacts(user_2).await.unwrap(), + &[ + Contact::Accepted { + user_id: user_1, + should_notify: false + }, + Contact::Accepted { + user_id: user_2, + should_notify: false + } + ] + ); + assert_eq!( + db.get_contacts(user_3).await.unwrap(), + &[ + Contact::Accepted { + user_id: user_1, + should_notify: false + }, + Contact::Accepted { + user_id: user_3, + should_notify: false + } + ], + ); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_invite_codes() { + let postgres = TestDb::postgres().await; + let db = postgres.db(); + let user1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); + + // Initially, user 1 has no invite code + assert_eq!(db.get_invite_code_for_user(user1).await.unwrap(), None); + + // Setting invite count to 0 when no code is assigned does not assign a new code + db.set_invite_count_for_user(user1, 0).await.unwrap(); + assert!(db.get_invite_code_for_user(user1).await.unwrap().is_none()); + + // User 1 creates an invite code that can be used twice. + db.set_invite_count_for_user(user1, 2).await.unwrap(); + let (invite_code, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); + assert_eq!(invite_count, 2); + + // User 2 redeems the invite code and becomes a contact of user 1. + let user2_invite = db + .create_invite_from_code(&invite_code, "u2@example.com") + .await + .unwrap(); + let (user2, inviter) = db + .create_user_from_invite( + &user2_invite, + NewUserParams { + github_login: "user2".into(), + invite_count: 7, + }, + ) + .await + .unwrap(); + let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); + assert_eq!(invite_count, 1); + assert_eq!(inviter, Some(user1)); + assert_eq!( + db.get_contacts(user1).await.unwrap(), + [ + Contact::Accepted { + user_id: user1, + should_notify: false + }, + Contact::Accepted { + user_id: user2, + should_notify: true + } + ] + ); + assert_eq!( + db.get_contacts(user2).await.unwrap(), + [ + Contact::Accepted { + user_id: user1, + should_notify: false + }, + Contact::Accepted { + user_id: user2, + should_notify: false + } + ] + ); + assert_eq!( + db.get_invite_code_for_user(user2).await.unwrap().unwrap().1, + 7 + ); + + // User 3 redeems the invite code and becomes a contact of user 1. + let user3_invite = db + .create_invite_from_code(&invite_code, "u3@example.com") + .await + .unwrap(); + let (user3, inviter) = db + .create_user_from_invite( + &user3_invite, + NewUserParams { + github_login: "user-3".into(), + invite_count: 3, + }, + ) + .await + .unwrap(); + let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); + assert_eq!(invite_count, 0); + assert_eq!(inviter, Some(user1)); + assert_eq!( + db.get_contacts(user1).await.unwrap(), + [ + Contact::Accepted { + user_id: user1, + should_notify: false + }, + Contact::Accepted { + user_id: user2, + should_notify: true + }, + Contact::Accepted { + user_id: user3, + should_notify: true + } + ] + ); + assert_eq!( + db.get_contacts(user3).await.unwrap(), + [ + Contact::Accepted { + user_id: user1, + should_notify: false + }, + Contact::Accepted { + user_id: user3, + should_notify: false + }, + ] + ); + assert_eq!( + db.get_invite_code_for_user(user3).await.unwrap().unwrap().1, + 3 + ); + + // Trying to reedem the code for the third time results in an error. + db.create_invite_from_code(&invite_code, "u4@example.com") + .await + .unwrap_err(); + + // Invite count can be updated after the code has been created. + db.set_invite_count_for_user(user1, 2).await.unwrap(); + let (latest_code, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); + assert_eq!(latest_code, invite_code); // Invite code doesn't change when we increment above 0 + assert_eq!(invite_count, 2); + + // User 4 can now redeem the invite code and becomes a contact of user 1. + let user4_invite = db + .create_invite_from_code(&invite_code, "u4@example.com") + .await + .unwrap(); + let (user4, _) = db + .create_user_from_invite( + &user4_invite, + NewUserParams { + github_login: "user-4".into(), + invite_count: 5, + }, + ) + .await + .unwrap(); + + let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); + assert_eq!(invite_count, 1); + assert_eq!( + db.get_contacts(user1).await.unwrap(), + [ + Contact::Accepted { + user_id: user1, + should_notify: false + }, + Contact::Accepted { + user_id: user2, + should_notify: true + }, + Contact::Accepted { + user_id: user3, + should_notify: true + }, + Contact::Accepted { + user_id: user4, + should_notify: true + } + ] + ); + assert_eq!( + db.get_contacts(user4).await.unwrap(), + [ + Contact::Accepted { + user_id: user1, + should_notify: false + }, + Contact::Accepted { + user_id: user4, + should_notify: false + }, + ] + ); + assert_eq!( + db.get_invite_code_for_user(user4).await.unwrap().unwrap().1, + 5 + ); + + // An existing user cannot redeem invite codes. + db.create_invite_from_code(&invite_code, "u2@example.com") + .await + .unwrap_err(); + let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); + assert_eq!(invite_count, 1); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_signups() { + let postgres = TestDb::postgres().await; + let db = postgres.db(); + + // people sign up on the waitlist + for i in 0..8 { + db.create_signup(Signup { + email_address: format!("person-{i}@example.com"), + platform_mac: true, + platform_linux: true, + platform_windows: false, + editor_features: vec!["speed".into()], + programming_languages: vec!["rust".into(), "c".into()], + }) + .await + .unwrap(); + } + + // retrieve the next batch of signup emails to send + let signups_batch1 = db.get_unsent_invites(3).await.unwrap(); + let addresses = signups_batch1 + .iter() + .map(|s| &s.email_address) + .collect::>(); + assert_eq!( + addresses, + &[ + "person-0@example.com", + "person-1@example.com", + "person-2@example.com" + ] + ); + assert_ne!( + signups_batch1[0].email_confirmation_code, + signups_batch1[1].email_confirmation_code + ); + + // the waitlist isn't updated until we record that the emails + // were successfully sent. + let signups_batch = db.get_unsent_invites(3).await.unwrap(); + assert_eq!(signups_batch, signups_batch1); + + // once the emails go out, we can retrieve the next batch + // of signups. + db.record_sent_invites(&signups_batch1).await.unwrap(); + let signups_batch2 = db.get_unsent_invites(3).await.unwrap(); + let addresses = signups_batch2 + .iter() + .map(|s| &s.email_address) + .collect::>(); + assert_eq!( + addresses, + &[ + "person-3@example.com", + "person-4@example.com", + "person-5@example.com" + ] + ); + + // user completes the signup process by providing their + // github account. + let (user_id, inviter_id) = db + .create_user_from_invite( + &Invite { + email_address: signups_batch1[0].email_address.clone(), + email_confirmation_code: signups_batch1[0].email_confirmation_code.clone(), + }, + NewUserParams { + github_login: "person-0".into(), + invite_count: 5, + }, + ) + .await + .unwrap(); + let user = db.get_user_by_id(user_id).await.unwrap().unwrap(); + assert!(inviter_id.is_none()); + assert_eq!(user.github_login, "person-0"); + assert_eq!(user.email_address.as_deref(), Some("person-0@example.com")); + assert_eq!(user.invite_count, 5); + + // cannot redeem the same signup again. + db.create_user_from_invite( + &Invite { + email_address: signups_batch1[0].email_address.clone(), + email_confirmation_code: signups_batch1[0].email_confirmation_code.clone(), + }, + NewUserParams { + github_login: "some-other-github_account".into(), + invite_count: 5, + }, + ) + .await + .unwrap_err(); + + // cannot redeem a signup with the wrong confirmation code. + db.create_user_from_invite( + &Invite { + email_address: signups_batch1[1].email_address.clone(), + email_confirmation_code: "the-wrong-code".to_string(), + }, + NewUserParams { + github_login: "person-1".into(), + invite_count: 5, + }, + ) + .await + .unwrap_err(); +} + +fn build_background_executor() -> Arc { + Deterministic::new(0).build_background() +} diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index 6b512d950f..1a4e4381c1 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -1,5 +1,5 @@ use crate::{ - db::{tests::TestDb, ProjectId, UserId}, + db::{ProjectId, TestDb, UserId}, rpc::{Executor, Server, Store}, AppState, }; @@ -4640,7 +4640,10 @@ async fn test_random_collaboration( let mut server = TestServer::start(cx.foreground(), cx.background()).await; let db = server.app_state.db.clone(); - let host_user_id = db.create_user("host", None, false).await.unwrap(); + let host_user_id = db + .create_user("host", "host@example.com", false) + .await + .unwrap(); let mut available_guests = vec![ "guest-1".to_string(), "guest-2".to_string(), @@ -4649,7 +4652,10 @@ async fn test_random_collaboration( ]; for username in &available_guests { - let guest_user_id = db.create_user(username, None, false).await.unwrap(); + let guest_user_id = db + .create_user(username, &format!("{username}@example.com"), false) + .await + .unwrap(); assert_eq!(*username, format!("guest-{}", guest_user_id)); server .app_state @@ -5157,7 +5163,7 @@ impl TestServer { } else { self.app_state .db - .create_user(name, None, false) + .create_user(name, &format!("{name}@example.com"), false) .await .unwrap() }; diff --git a/crates/collab/src/main.rs b/crates/collab/src/main.rs index 2c2c6a94f4..272d52cc95 100644 --- a/crates/collab/src/main.rs +++ b/crates/collab/src/main.rs @@ -4,6 +4,8 @@ mod db; mod env; mod rpc; +#[cfg(test)] +mod db_tests; #[cfg(test)] mod integration_tests; diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index dab7df3e67..4fc022995f 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -541,27 +541,30 @@ impl Server { pub async fn invite_code_redeemed( self: &Arc, - code: &str, + inviter_id: UserId, invitee_id: UserId, ) -> Result<()> { - let user = self.app_state.db.get_user_for_invite_code(code).await?; - let store = self.store().await; - let invitee_contact = store.contact_for_user(invitee_id, true); - for connection_id in store.connection_ids_for_user(user.id) { - self.peer.send( - connection_id, - proto::UpdateContacts { - contacts: vec![invitee_contact.clone()], - ..Default::default() - }, - )?; - self.peer.send( - connection_id, - proto::UpdateInviteInfo { - url: format!("{}{}", self.app_state.invite_link_prefix, code), - count: user.invite_count as u32, - }, - )?; + if let Some(user) = self.app_state.db.get_user_by_id(inviter_id).await? { + if let Some(code) = &user.invite_code { + let store = self.store().await; + let invitee_contact = store.contact_for_user(invitee_id, true); + for connection_id in store.connection_ids_for_user(inviter_id) { + self.peer.send( + connection_id, + proto::UpdateContacts { + contacts: vec![invitee_contact.clone()], + ..Default::default() + }, + )?; + self.peer.send( + connection_id, + proto::UpdateInviteInfo { + url: format!("{}{}", self.app_state.invite_link_prefix, &code), + count: user.invite_count as u32, + }, + )?; + } + } } Ok(()) } From 3dd8845bd88f1fafc7d79f712113069c0a98ef11 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 16 Sep 2022 15:37:19 -0700 Subject: [PATCH 03/22] Add waitlist summary API --- crates/collab/src/api.rs | 9 ++++++++- crates/collab/src/db.rs | 37 +++++++++++++++++++++++++++++++++++ crates/collab/src/db_tests.rs | 25 +++++++++++++++++++++-- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 26521ceb27..504880f0a3 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -1,6 +1,6 @@ use crate::{ auth, - db::{Invite, NewUserParams, ProjectId, Signup, User, UserId}, + db::{Invite, NewUserParams, ProjectId, Signup, User, UserId, WaitlistSummary}, rpc::{self, ResultExt}, AppState, Error, Result, }; @@ -46,6 +46,7 @@ pub fn routes(rpc_server: &Arc, state: Arc) -> Router>, +) -> Result> { + Ok(Json(app.db.get_waitlist_summary().await?)) +} + #[derive(Deserialize)] pub struct CreateInviteFromCodeParams { invite_code: String, diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 9c1ab84570..1509b15cb2 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -35,6 +35,7 @@ pub trait Db: Send + Sync { async fn create_invite_from_code(&self, code: &str, email_address: &str) -> Result; async fn create_signup(&self, signup: Signup) -> Result<()>; + async fn get_waitlist_summary(&self) -> Result; async fn get_unsent_invites(&self, count: usize) -> Result>; async fn record_sent_invites(&self, invites: &[Invite]) -> Result<()>; async fn create_user_from_invite( @@ -384,6 +385,26 @@ impl Db for PostgresDb { Ok(()) } + async fn get_waitlist_summary(&self) -> Result { + Ok(sqlx::query_as( + " + SELECT + COUNT(*) as count, + COALESCE(SUM(CASE WHEN platform_linux THEN 1 ELSE 0 END), 0) as linux_count, + COALESCE(SUM(CASE WHEN platform_mac THEN 1 ELSE 0 END), 0) as mac_count, + COALESCE(SUM(CASE WHEN platform_windows THEN 1 ELSE 0 END), 0) as windows_count + FROM ( + SELECT * + FROM signups + WHERE + NOT email_confirmation_sent + ) AS unsent + ", + ) + .fetch_one(&self.pool) + .await?) + } + async fn get_unsent_invites(&self, count: usize) -> Result> { Ok(sqlx::query_as( " @@ -1630,6 +1651,18 @@ pub struct Signup { pub programming_languages: Vec, } +#[derive(Clone, Debug, PartialEq, Deserialize, Serialize, FromRow)] +pub struct WaitlistSummary { + #[sqlx(default)] + pub count: i64, + #[sqlx(default)] + pub linux_count: i64, + #[sqlx(default)] + pub mac_count: i64, + #[sqlx(default)] + pub windows_count: i64, +} + #[derive(FromRow, PartialEq, Debug, Serialize, Deserialize)] pub struct Invite { pub email_address: String, @@ -1812,6 +1845,10 @@ mod test { unimplemented!() } + async fn get_waitlist_summary(&self) -> Result { + unimplemented!() + } + async fn get_unsent_invites(&self, _count: usize) -> Result> { unimplemented!() } diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index aa9a0b6995..4ff8bbd0f6 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -966,8 +966,8 @@ async fn test_signups() { db.create_signup(Signup { email_address: format!("person-{i}@example.com"), platform_mac: true, - platform_linux: true, - platform_windows: false, + platform_linux: i % 2 == 0, + platform_windows: i % 4 == 0, editor_features: vec!["speed".into()], programming_languages: vec!["rust".into(), "c".into()], }) @@ -975,6 +975,16 @@ async fn test_signups() { .unwrap(); } + assert_eq!( + db.get_waitlist_summary().await.unwrap(), + WaitlistSummary { + count: 8, + mac_count: 8, + linux_count: 4, + windows_count: 2, + } + ); + // retrieve the next batch of signup emails to send let signups_batch1 = db.get_unsent_invites(3).await.unwrap(); let addresses = signups_batch1 @@ -1016,6 +1026,17 @@ async fn test_signups() { ] ); + // the sent invites are excluded from the summary. + assert_eq!( + db.get_waitlist_summary().await.unwrap(), + WaitlistSummary { + count: 5, + mac_count: 5, + linux_count: 2, + windows_count: 1, + } + ); + // user completes the signup process by providing their // github account. let (user_id, inviter_id) = db From 963ced1dd8f46dfc8f50c1cccf7799652008a6f6 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 16 Sep 2022 15:45:10 -0700 Subject: [PATCH 04/22] Preserve metrics_id from signup to user record --- .../migrations/20220913211150_create_signups.down.sql | 2 ++ .../migrations/20220913211150_create_signups.up.sql | 3 +++ crates/collab/src/db.rs | 9 +++++---- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/collab/migrations/20220913211150_create_signups.down.sql b/crates/collab/migrations/20220913211150_create_signups.down.sql index 6ef51842c9..14b4e43cea 100644 --- a/crates/collab/migrations/20220913211150_create_signups.down.sql +++ b/crates/collab/migrations/20220913211150_create_signups.down.sql @@ -4,3 +4,5 @@ ALTER TABLE users DROP COLUMN metrics_id; DROP SEQUENCE metrics_id_seq; + +DROP INDEX index_users_on_email_address; \ No newline at end of file diff --git a/crates/collab/migrations/20220913211150_create_signups.up.sql b/crates/collab/migrations/20220913211150_create_signups.up.sql index 9acb313fd6..6c9380275d 100644 --- a/crates/collab/migrations/20220913211150_create_signups.up.sql +++ b/crates/collab/migrations/20220913211150_create_signups.up.sql @@ -25,3 +25,6 @@ CREATE INDEX "index_signups_on_email_confirmation_sent" ON "signups" ("email_con ALTER TABLE "users" ADD "metrics_id" INTEGER DEFAULT nextval('metrics_id_seq'); + +UPDATE users +SET metrics_id = nextval('metrics_id_seq'); diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 1509b15cb2..c89a8ba0fc 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -448,9 +448,9 @@ impl Db for PostgresDb { ) -> Result<(UserId, Option)> { let mut tx = self.pool.begin().await?; - let (signup_id, inviting_user_id): (i32, Option) = sqlx::query_as( + let (signup_id, metrics_id, inviting_user_id): (i32, i32, Option) = sqlx::query_as( " - SELECT id, inviting_user_id + SELECT id, metrics_id, inviting_user_id FROM signups WHERE email_address = $1 AND @@ -467,9 +467,9 @@ impl Db for PostgresDb { let user_id: UserId = sqlx::query_scalar( " INSERT INTO users - (email_address, github_login, admin, invite_count, invite_code) + (email_address, github_login, admin, invite_count, invite_code, metrics_id) VALUES - ($1, $2, 'f', $3, $4) + ($1, $2, 'f', $3, $4, $5) RETURNING id ", ) @@ -477,6 +477,7 @@ impl Db for PostgresDb { .bind(&user.github_login) .bind(&user.invite_count) .bind(random_invite_code()) + .bind(metrics_id) .fetch_one(&mut tx) .await?; From e77263a3c7d7db9429f8da235f3f951bf342fdd9 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 19 Sep 2022 14:34:37 -0700 Subject: [PATCH 05/22] Remove bulk user creation admin API --- crates/collab/src/api.rs | 37 -------------------- crates/collab/src/db.rs | 42 +---------------------- crates/collab/src/db_tests.rs | 64 ----------------------------------- 3 files changed, 1 insertion(+), 142 deletions(-) diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 504880f0a3..a390b4392a 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -30,7 +30,6 @@ pub fn routes(rpc_server: &Arc, state: Arc) -> Router, -} - -#[derive(Deserialize)] -struct CreateUsersEntry { - github_login: String, - email_address: String, - invite_count: usize, -} - -async fn create_users( - Json(params): Json, - Extension(app): Extension>, -) -> Result>> { - let user_ids = app - .db - .create_users( - params - .users - .into_iter() - .map(|params| { - ( - params.github_login, - params.email_address, - params.invite_count, - ) - }) - .collect(), - ) - .await?; - let users = app.db.get_users_by_ids(user_ids).await?; - Ok(Json(users)) -} - #[derive(Debug, Deserialize)] struct GetUsersWithNoInvites { invited_by_another_user: bool, diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index c89a8ba0fc..85f13a6a11 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -6,7 +6,7 @@ use collections::HashMap; use futures::StreamExt; use serde::{Deserialize, Serialize}; pub use sqlx::postgres::PgPoolOptions as DbOptions; -use sqlx::{types::Uuid, FromRow, QueryBuilder, Row}; +use sqlx::{types::Uuid, FromRow, QueryBuilder}; use std::{cmp, ops::Range, time::Duration}; use time::{OffsetDateTime, PrimitiveDateTime}; @@ -19,7 +19,6 @@ pub trait Db: Send + Sync { admin: bool, ) -> Result; async fn get_all_users(&self, page: u32, limit: u32) -> Result>; - async fn create_users(&self, users: Vec<(String, String, usize)>) -> Result>; async fn fuzzy_search_users(&self, query: &str, limit: u32) -> Result>; async fn get_user_by_id(&self, id: UserId) -> Result>; async fn get_users_by_ids(&self, ids: Vec) -> Result>; @@ -225,41 +224,6 @@ impl Db for PostgresDb { .await?) } - async fn create_users(&self, users: Vec<(String, String, usize)>) -> Result> { - let mut query = QueryBuilder::new( - "INSERT INTO users (github_login, email_address, admin, invite_code, invite_count)", - ); - query.push_values( - users, - |mut query, (github_login, email_address, invite_count)| { - query - .push_bind(github_login) - .push_bind(email_address) - .push_bind(false) - .push_bind(random_invite_code()) - .push_bind(invite_count as i32); - }, - ); - query.push( - " - ON CONFLICT (github_login) DO UPDATE SET - github_login = excluded.github_login, - invite_count = excluded.invite_count, - invite_code = CASE WHEN users.invite_code IS NULL - THEN excluded.invite_code - ELSE users.invite_code - END - RETURNING id - ", - ); - - let rows = query.build().fetch_all(&self.pool).await?; - Ok(rows - .into_iter() - .filter_map(|row| row.try_get::(0).ok()) - .collect()) - } - async fn fuzzy_search_users(&self, name_query: &str, limit: u32) -> Result> { let like_string = Self::fuzzy_like_string(name_query); let query = " @@ -1789,10 +1753,6 @@ mod test { unimplemented!() } - async fn create_users(&self, _users: Vec<(String, String, usize)>) -> Result> { - unimplemented!() - } - async fn fuzzy_search_users(&self, _: &str, _: u32) -> Result> { unimplemented!() } diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index 4ff8bbd0f6..16eba2fb22 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -55,70 +55,6 @@ async fn test_get_users_by_ids() { } } -#[tokio::test(flavor = "multi_thread")] -async fn test_create_users() { - let db = TestDb::postgres().await; - let db = db.db(); - - // Create the first batch of users, ensuring invite counts are assigned - // correctly and the respective invite codes are unique. - let user_ids_batch_1 = db - .create_users(vec![ - ("user1".to_string(), "hi@user1.com".to_string(), 5), - ("user2".to_string(), "hi@user2.com".to_string(), 4), - ("user3".to_string(), "hi@user3.com".to_string(), 3), - ]) - .await - .unwrap(); - assert_eq!(user_ids_batch_1.len(), 3); - - let users = db.get_users_by_ids(user_ids_batch_1.clone()).await.unwrap(); - assert_eq!(users.len(), 3); - assert_eq!(users[0].github_login, "user1"); - assert_eq!(users[0].email_address.as_deref(), Some("hi@user1.com")); - assert_eq!(users[0].invite_count, 5); - assert_eq!(users[1].github_login, "user2"); - assert_eq!(users[1].email_address.as_deref(), Some("hi@user2.com")); - assert_eq!(users[1].invite_count, 4); - assert_eq!(users[2].github_login, "user3"); - assert_eq!(users[2].email_address.as_deref(), Some("hi@user3.com")); - assert_eq!(users[2].invite_count, 3); - - let invite_code_1 = users[0].invite_code.clone().unwrap(); - let invite_code_2 = users[1].invite_code.clone().unwrap(); - let invite_code_3 = users[2].invite_code.clone().unwrap(); - assert_ne!(invite_code_1, invite_code_2); - assert_ne!(invite_code_1, invite_code_3); - assert_ne!(invite_code_2, invite_code_3); - - // Create the second batch of users and include a user that is already in the database, ensuring - // the invite count for the existing user is updated without changing their invite code. - let user_ids_batch_2 = db - .create_users(vec![ - ("user2".to_string(), "hi@user2.com".to_string(), 10), - ("user4".to_string(), "hi@user4.com".to_string(), 2), - ]) - .await - .unwrap(); - assert_eq!(user_ids_batch_2.len(), 2); - assert_eq!(user_ids_batch_2[0], user_ids_batch_1[1]); - - let users = db.get_users_by_ids(user_ids_batch_2).await.unwrap(); - assert_eq!(users.len(), 2); - assert_eq!(users[0].github_login, "user2"); - assert_eq!(users[0].email_address.as_deref(), Some("hi@user2.com")); - assert_eq!(users[0].invite_count, 10); - assert_eq!(users[0].invite_code, Some(invite_code_2.clone())); - assert_eq!(users[1].github_login, "user4"); - assert_eq!(users[1].email_address.as_deref(), Some("hi@user4.com")); - assert_eq!(users[1].invite_count, 2); - - let invite_code_4 = users[1].invite_code.clone().unwrap(); - assert_ne!(invite_code_4, invite_code_1); - assert_ne!(invite_code_4, invite_code_2); - assert_ne!(invite_code_4, invite_code_3); -} - #[tokio::test(flavor = "multi_thread")] async fn test_worktree_extensions() { let test_db = TestDb::postgres().await; From 9886259b3aea123000307c5e11e089ed98a8924d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 20 Sep 2022 09:44:56 -0700 Subject: [PATCH 06/22] Start storing users' github user id --- .../20220913211150_create_signups.down.sql | 4 +- .../20220913211150_create_signups.up.sql | 5 +- crates/collab/src/api.rs | 14 +- crates/collab/src/db.rs | 25 +- crates/collab/src/db_tests.rs | 226 ++++++++++++++++-- crates/collab/src/integration_tests.rs | 30 ++- 6 files changed, 262 insertions(+), 42 deletions(-) diff --git a/crates/collab/migrations/20220913211150_create_signups.down.sql b/crates/collab/migrations/20220913211150_create_signups.down.sql index 14b4e43cea..ec02ac3322 100644 --- a/crates/collab/migrations/20220913211150_create_signups.down.sql +++ b/crates/collab/migrations/20220913211150_create_signups.down.sql @@ -1,8 +1,10 @@ DROP TABLE signups; ALTER TABLE users + DROP COLUMN github_user_id, DROP COLUMN metrics_id; DROP SEQUENCE metrics_id_seq; -DROP INDEX index_users_on_email_address; \ No newline at end of file +DROP INDEX index_users_on_email_address; +DROP INDEX index_users_on_github_user_id; \ No newline at end of file diff --git a/crates/collab/migrations/20220913211150_create_signups.up.sql b/crates/collab/migrations/20220913211150_create_signups.up.sql index 6c9380275d..3de683c58e 100644 --- a/crates/collab/migrations/20220913211150_create_signups.up.sql +++ b/crates/collab/migrations/20220913211150_create_signups.up.sql @@ -19,12 +19,15 @@ CREATE TABLE IF NOT EXISTS "signups" ( "programming_languages" VARCHAR[] ); -CREATE INDEX "index_users_on_email_address" ON "users" ("email_address"); CREATE UNIQUE INDEX "index_signups_on_email_address" ON "signups" ("email_address"); CREATE INDEX "index_signups_on_email_confirmation_sent" ON "signups" ("email_confirmation_sent"); ALTER TABLE "users" + ADD "github_user_id" INTEGER, ADD "metrics_id" INTEGER DEFAULT nextval('metrics_id_seq'); +CREATE INDEX "index_users_on_email_address" ON "users" ("email_address"); +CREATE INDEX "index_users_on_github_user_id" ON "users" ("github_user_id"); + UPDATE users SET metrics_id = nextval('metrics_id_seq'); diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index a390b4392a..de8ec44c78 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -112,9 +112,11 @@ async fn get_users( #[derive(Deserialize, Debug)] struct CreateUserParams { + github_user_id: i32, github_login: String, email_address: String, email_confirmation_code: Option, + #[serde(default)] invite_count: i32, } @@ -123,6 +125,11 @@ async fn create_user( Extension(app): Extension>, Extension(rpc_server): Extension>, ) -> Result> { + let user = NewUserParams { + github_login: params.github_login, + github_user_id: params.github_user_id, + invite_count: params.invite_count, + }; let (user_id, inviter_id) = // Creating a user via the normal signup process if let Some(email_confirmation_code) = params.email_confirmation_code { @@ -132,10 +139,7 @@ async fn create_user( email_address: params.email_address, email_confirmation_code, }, - NewUserParams { - github_login: params.github_login, - invite_count: params.invite_count, - }, + user, ) .await? } @@ -143,7 +147,7 @@ async fn create_user( else { ( app.db - .create_user(¶ms.github_login, ¶ms.email_address, false) + .create_user(¶ms.email_address, false, user) .await?, None, ) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 85f13a6a11..f31defa577 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -14,9 +14,9 @@ use time::{OffsetDateTime, PrimitiveDateTime}; pub trait Db: Send + Sync { async fn create_user( &self, - github_login: &str, email_address: &str, admin: bool, + params: NewUserParams, ) -> Result; async fn get_all_users(&self, page: u32, limit: u32) -> Result>; async fn fuzzy_search_users(&self, query: &str, limit: u32) -> Result>; @@ -196,19 +196,20 @@ impl Db for PostgresDb { async fn create_user( &self, - github_login: &str, email_address: &str, admin: bool, + params: NewUserParams, ) -> Result { let query = " - INSERT INTO users (github_login, email_address, admin) - VALUES ($1, $2, $3) + INSERT INTO users (email_address, github_login, github_user_id, admin) + VALUES ($1, $2, $3, $4) ON CONFLICT (github_login) DO UPDATE SET github_login = excluded.github_login RETURNING id "; Ok(sqlx::query_scalar(query) - .bind(github_login) .bind(email_address) + .bind(params.github_login) + .bind(params.github_user_id) .bind(admin) .fetch_one(&self.pool) .await @@ -431,14 +432,15 @@ impl Db for PostgresDb { let user_id: UserId = sqlx::query_scalar( " INSERT INTO users - (email_address, github_login, admin, invite_count, invite_code, metrics_id) + (email_address, github_login, github_user_id, admin, invite_count, invite_code, metrics_id) VALUES - ($1, $2, 'f', $3, $4, $5) + ($1, $2, $3, 'f', $4, $5, $6) RETURNING id ", ) .bind(&invite.email_address) .bind(&user.github_login) + .bind(&user.github_user_id) .bind(&user.invite_count) .bind(random_invite_code()) .bind(metrics_id) @@ -1508,6 +1510,7 @@ id_type!(UserId); pub struct User { pub id: UserId, pub github_login: String, + pub github_user_id: i32, pub email_address: Option, pub admin: bool, pub invite_code: Option, @@ -1637,6 +1640,7 @@ pub struct Invite { #[derive(Debug, Serialize, Deserialize)] pub struct NewUserParams { pub github_login: String, + pub github_user_id: i32, pub invite_count: i32, } @@ -1719,16 +1723,16 @@ mod test { impl Db for FakeDb { async fn create_user( &self, - github_login: &str, email_address: &str, admin: bool, + params: NewUserParams, ) -> Result { self.background.simulate_random_delay().await; let mut users = self.users.lock(); if let Some(user) = users .values() - .find(|user| user.github_login == github_login) + .find(|user| user.github_login == params.github_login) { Ok(user.id) } else { @@ -1737,7 +1741,8 @@ mod test { user_id, User { id: user_id, - github_login: github_login.to_string(), + github_login: params.github_login, + github_user_id: params.github_user_id, email_address: Some(email_address.to_string()), admin, invite_code: None, diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index 16eba2fb22..87033fab38 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -12,10 +12,54 @@ async fn test_get_users_by_ids() { ] { let db = test_db.db(); - let user1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); - let user2 = db.create_user("u2", "u2@example.com", false).await.unwrap(); - let user3 = db.create_user("u3", "u3@example.com", false).await.unwrap(); - let user4 = db.create_user("u4", "u4@example.com", false).await.unwrap(); + let user1 = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user2 = db + .create_user( + "u2@example.com", + false, + NewUserParams { + github_login: "u2".into(), + github_user_id: 2, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user3 = db + .create_user( + "u3@example.com", + false, + NewUserParams { + github_login: "u3".into(), + github_user_id: 3, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user4 = db + .create_user( + "u4@example.com", + false, + NewUserParams { + github_login: "u4".into(), + github_user_id: 4, + invite_count: 0, + }, + ) + .await + .unwrap(); assert_eq!( db.get_users_by_ids(vec![user1, user2, user3, user4]) @@ -25,6 +69,7 @@ async fn test_get_users_by_ids() { User { id: user1, github_login: "u1".to_string(), + github_user_id: 1, email_address: Some("u1@example.com".to_string()), admin: false, ..Default::default() @@ -32,6 +77,7 @@ async fn test_get_users_by_ids() { User { id: user2, github_login: "u2".to_string(), + github_user_id: 2, email_address: Some("u2@example.com".to_string()), admin: false, ..Default::default() @@ -39,6 +85,7 @@ async fn test_get_users_by_ids() { User { id: user3, github_login: "u3".to_string(), + github_user_id: 3, email_address: Some("u3@example.com".to_string()), admin: false, ..Default::default() @@ -46,6 +93,7 @@ async fn test_get_users_by_ids() { User { id: user4, github_login: "u4".to_string(), + github_user_id: 4, email_address: Some("u4@example.com".to_string()), admin: false, ..Default::default() @@ -60,7 +108,18 @@ async fn test_worktree_extensions() { let test_db = TestDb::postgres().await; let db = test_db.db(); - let user = db.create_user("u1", "u1@example.com", false).await.unwrap(); + let user = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); let project = db.register_project(user).await.unwrap(); db.update_worktree_extensions(project, 100, Default::default()) @@ -120,9 +179,42 @@ async fn test_user_activity() { let test_db = TestDb::postgres().await; let db = test_db.db(); - let user_1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); - let user_2 = db.create_user("u2", "u2@example.com", false).await.unwrap(); - let user_3 = db.create_user("u3", "u3@example.com", false).await.unwrap(); + let user_1 = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user_2 = db + .create_user( + "u2@example.com", + false, + NewUserParams { + github_login: "u2".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user_3 = db + .create_user( + "u3@example.com", + false, + NewUserParams { + github_login: "u3".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); let project_1 = db.register_project(user_1).await.unwrap(); db.update_worktree_extensions( project_1, @@ -340,7 +432,18 @@ async fn test_recent_channel_messages() { TestDb::fake(build_background_executor()), ] { let db = test_db.db(); - let user = db.create_user("u", "u@example.com", false).await.unwrap(); + let user = db + .create_user( + "u@example.com", + false, + NewUserParams { + github_login: "u".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap(); let org = db.create_org("org", "org").await.unwrap(); let channel = db.create_org_channel(org, "channel").await.unwrap(); for i in 0..10 { @@ -373,7 +476,18 @@ async fn test_channel_message_nonces() { TestDb::fake(build_background_executor()), ] { let db = test_db.db(); - let user = db.create_user("u", "u@example.com", false).await.unwrap(); + let user = db + .create_user( + "user@example.com", + false, + NewUserParams { + github_login: "user".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap(); let org = db.create_org("org", "org").await.unwrap(); let channel = db.create_org_channel(org, "channel").await.unwrap(); @@ -404,7 +518,18 @@ async fn test_channel_message_nonces() { async fn test_create_access_tokens() { let test_db = TestDb::postgres().await; let db = test_db.db(); - let user = db.create_user("u1", "u1@example.com", false).await.unwrap(); + let user = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap(); db.create_access_token_hash(user, "h1", 3).await.unwrap(); db.create_access_token_hash(user, "h2", 3).await.unwrap(); @@ -443,7 +568,7 @@ fn test_fuzzy_like_string() { async fn test_fuzzy_search_users() { let test_db = TestDb::postgres().await; let db = test_db.db(); - for github_login in [ + for (i, github_login) in [ "California", "colorado", "oregon", @@ -451,10 +576,21 @@ async fn test_fuzzy_search_users() { "florida", "delaware", "rhode-island", - ] { - db.create_user(github_login, &format!("{github_login}@example.com"), false) - .await - .unwrap(); + ] + .into_iter() + .enumerate() + { + db.create_user( + &format!("{github_login}@example.com"), + false, + NewUserParams { + github_login: github_login.into(), + github_user_id: i as i32, + invite_count: 0, + }, + ) + .await + .unwrap(); } assert_eq!( @@ -484,9 +620,42 @@ async fn test_add_contacts() { ] { let db = test_db.db(); - let user_1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); - let user_2 = db.create_user("u2", "u2@example.com", false).await.unwrap(); - let user_3 = db.create_user("u3", "u3@example.com", false).await.unwrap(); + let user_1 = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user_2 = db + .create_user( + "u2@example.com", + false, + NewUserParams { + github_login: "u2".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user_3 = db + .create_user( + "u3@example.com", + false, + NewUserParams { + github_login: "u3".into(), + github_user_id: 2, + invite_count: 0, + }, + ) + .await + .unwrap(); // User starts with no contacts assert_eq!( @@ -700,7 +869,18 @@ async fn test_add_contacts() { async fn test_invite_codes() { let postgres = TestDb::postgres().await; let db = postgres.db(); - let user1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); + let user1 = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); // Initially, user 1 has no invite code assert_eq!(db.get_invite_code_for_user(user1).await.unwrap(), None); @@ -724,6 +904,7 @@ async fn test_invite_codes() { &user2_invite, NewUserParams { github_login: "user2".into(), + github_user_id: 2, invite_count: 7, }, ) @@ -773,6 +954,7 @@ async fn test_invite_codes() { &user3_invite, NewUserParams { github_login: "user-3".into(), + github_user_id: 3, invite_count: 3, }, ) @@ -837,6 +1019,7 @@ async fn test_invite_codes() { &user4_invite, NewUserParams { github_login: "user-4".into(), + github_user_id: 4, invite_count: 5, }, ) @@ -983,6 +1166,7 @@ async fn test_signups() { }, NewUserParams { github_login: "person-0".into(), + github_user_id: 0, invite_count: 5, }, ) @@ -1002,6 +1186,7 @@ async fn test_signups() { }, NewUserParams { github_login: "some-other-github_account".into(), + github_user_id: 1, invite_count: 5, }, ) @@ -1016,6 +1201,7 @@ async fn test_signups() { }, NewUserParams { github_login: "person-1".into(), + github_user_id: 2, invite_count: 5, }, ) diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index 1a4e4381c1..94811b0951 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -1,5 +1,5 @@ use crate::{ - db::{ProjectId, TestDb, UserId}, + db::{NewUserParams, ProjectId, TestDb, UserId}, rpc::{Executor, Server, Store}, AppState, }; @@ -4641,7 +4641,15 @@ async fn test_random_collaboration( let mut server = TestServer::start(cx.foreground(), cx.background()).await; let db = server.app_state.db.clone(); let host_user_id = db - .create_user("host", "host@example.com", false) + .create_user( + "host@example.com", + false, + NewUserParams { + github_login: "host".into(), + github_user_id: 0, + invite_count: 0, + }, + ) .await .unwrap(); let mut available_guests = vec![ @@ -4651,9 +4659,17 @@ async fn test_random_collaboration( "guest-4".to_string(), ]; - for username in &available_guests { + for (ix, username) in available_guests.iter().enumerate() { let guest_user_id = db - .create_user(username, &format!("{username}@example.com"), false) + .create_user( + &format!("{username}@example.com"), + false, + NewUserParams { + github_login: username.into(), + github_user_id: ix as i32, + invite_count: 0, + }, + ) .await .unwrap(); assert_eq!(*username, format!("guest-{}", guest_user_id)); @@ -5163,7 +5179,11 @@ impl TestServer { } else { self.app_state .db - .create_user(name, &format!("{name}@example.com"), false) + .create_user(&format!("{name}@example.com"), false, NewUserParams { + github_login: name.into(), + github_user_id: 0, + invite_count: 0, + }) .await .unwrap() }; From 1877fc234b2add3549d144f1c6ae101a11a36b5a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 20 Sep 2022 15:40:56 -0700 Subject: [PATCH 07/22] Update user retrieval API to take both github user id and github login --- crates/collab/src/api.rs | 39 ++++++------ crates/collab/src/db.rs | 85 ++++++++++++++++++++++---- crates/collab/src/db_tests.rs | 58 ++++++++++++++++++ crates/collab/src/integration_tests.rs | 20 ++++-- crates/collab/src/rpc.rs | 2 +- 5 files changed, 165 insertions(+), 39 deletions(-) diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index de8ec44c78..73293e0b2c 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -25,10 +25,7 @@ use tracing::instrument; pub fn routes(rpc_server: &Arc, state: Arc) -> Router { Router::new() .route("/users", get(get_users).post(create_user)) - .route( - "/users/:id", - put(update_user).delete(destroy_user).get(get_user), - ) + .route("/users/:id", put(update_user).delete(destroy_user)) .route("/users/:id/access_tokens", post(create_access_token)) .route("/users_with_no_invites", get(get_users_with_no_invites)) .route("/invite_codes/:code", get(get_user_for_invite_code)) @@ -90,6 +87,8 @@ pub async fn validate_api_token(req: Request, next: Next) -> impl IntoR #[derive(Debug, Deserialize)] struct GetUsersQueryParams { + github_user_id: Option, + github_login: Option, query: Option, page: Option, limit: Option, @@ -99,6 +98,14 @@ async fn get_users( Query(params): Query, Extension(app): Extension>, ) -> Result>> { + if let Some(github_login) = ¶ms.github_login { + let user = app + .db + .get_user_by_github_account(github_login, params.github_user_id) + .await?; + return Ok(Json(Vec::from_iter(user))); + } + let limit = params.limit.unwrap_or(100); let users = if let Some(query) = params.query { app.db.fuzzy_search_users(&query, limit).await? @@ -205,18 +212,6 @@ async fn destroy_user( Ok(()) } -async fn get_user( - Path(login): Path, - Extension(app): Extension>, -) -> Result> { - let user = app - .db - .get_user_by_github_login(&login) - .await? - .ok_or_else(|| Error::Http(StatusCode::NOT_FOUND, "User not found".to_string()))?; - Ok(Json(user)) -} - #[derive(Debug, Deserialize)] struct GetUsersWithNoInvites { invited_by_another_user: bool, @@ -351,22 +346,24 @@ struct CreateAccessTokenResponse { } async fn create_access_token( - Path(login): Path, + Path(user_id): Path, Query(params): Query, Extension(app): Extension>, ) -> Result> { - // request.require_token().await?; - let user = app .db - .get_user_by_github_login(&login) + .get_user_by_id(user_id) .await? .ok_or_else(|| anyhow!("user not found"))?; let mut user_id = user.id; if let Some(impersonate) = params.impersonate { if user.admin { - if let Some(impersonated_user) = app.db.get_user_by_github_login(&impersonate).await? { + if let Some(impersonated_user) = app + .db + .get_user_by_github_account(&impersonate, None) + .await? + { user_id = impersonated_user.id; } else { return Err(Error::Http( diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index f31defa577..70dc0c4e5b 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -23,7 +23,11 @@ pub trait Db: Send + Sync { async fn get_user_by_id(&self, id: UserId) -> Result>; async fn get_users_by_ids(&self, ids: Vec) -> Result>; async fn get_users_with_no_invites(&self, invited_by_another_user: bool) -> Result>; - async fn get_user_by_github_login(&self, github_login: &str) -> Result>; + async fn get_user_by_github_account( + &self, + github_login: &str, + github_user_id: Option, + ) -> Result>; async fn set_user_is_admin(&self, id: UserId, is_admin: bool) -> Result<()>; async fn set_user_connected_once(&self, id: UserId, connected_once: bool) -> Result<()>; async fn destroy_user(&self, id: UserId) -> Result<()>; @@ -274,12 +278,53 @@ impl Db for PostgresDb { Ok(sqlx::query_as(&query).fetch_all(&self.pool).await?) } - async fn get_user_by_github_login(&self, github_login: &str) -> Result> { - let query = "SELECT * FROM users WHERE github_login = $1 LIMIT 1"; - Ok(sqlx::query_as(query) + async fn get_user_by_github_account( + &self, + github_login: &str, + github_user_id: Option, + ) -> Result> { + if let Some(github_user_id) = github_user_id { + let mut user = sqlx::query_as::<_, User>( + " + UPDATE users + SET github_login = $1 + WHERE github_user_id = $2 + RETURNING * + ", + ) + .bind(github_login) + .bind(github_user_id) + .fetch_optional(&self.pool) + .await?; + + if user.is_none() { + user = sqlx::query_as::<_, User>( + " + UPDATE users + SET github_user_id = $1 + WHERE github_login = $2 + RETURNING * + ", + ) + .bind(github_user_id) + .bind(github_login) + .fetch_optional(&self.pool) + .await?; + } + + Ok(user) + } else { + Ok(sqlx::query_as( + " + SELECT * FROM users + WHERE github_login = $1 + LIMIT 1 + ", + ) .bind(github_login) .fetch_optional(&self.pool) .await?) + } } async fn set_user_is_admin(&self, id: UserId, is_admin: bool) -> Result<()> { @@ -1777,14 +1822,32 @@ mod test { unimplemented!() } - async fn get_user_by_github_login(&self, github_login: &str) -> Result> { + async fn get_user_by_github_account( + &self, + github_login: &str, + github_user_id: Option, + ) -> Result> { self.background.simulate_random_delay().await; - Ok(self - .users - .lock() - .values() - .find(|user| user.github_login == github_login) - .cloned()) + if let Some(github_user_id) = github_user_id { + for user in self.users.lock().values_mut() { + if user.github_user_id == github_user_id { + user.github_login = github_login.into(); + return Ok(Some(user.clone())); + } + if user.github_login == github_login { + user.github_user_id = github_user_id; + return Ok(Some(user.clone())); + } + } + Ok(None) + } else { + Ok(self + .users + .lock() + .values() + .find(|user| user.github_login == github_login) + .cloned()) + } } async fn set_user_is_admin(&self, _id: UserId, _is_admin: bool) -> Result<()> { diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index 87033fab38..49ac053fd8 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -103,6 +103,64 @@ async fn test_get_users_by_ids() { } } +#[tokio::test(flavor = "multi_thread")] +async fn test_get_user_by_github_account() { + for test_db in [ + TestDb::postgres().await, + TestDb::fake(build_background_executor()), + ] { + let db = test_db.db(); + let user_id1 = db + .create_user( + "user1@example.com", + false, + NewUserParams { + github_login: "login1".into(), + github_user_id: 101, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user_id2 = db + .create_user( + "user2@example.com", + false, + NewUserParams { + github_login: "login2".into(), + github_user_id: 102, + invite_count: 0, + }, + ) + .await + .unwrap(); + + let user = db + .get_user_by_github_account("login1", None) + .await + .unwrap() + .unwrap(); + assert_eq!(user.id, user_id1); + assert_eq!(&user.github_login, "login1"); + assert_eq!(user.github_user_id, 101); + + assert!(db + .get_user_by_github_account("non-existent-login", None) + .await + .unwrap() + .is_none()); + + let user = db + .get_user_by_github_account("the-new-login2", Some(102)) + .await + .unwrap() + .unwrap(); + assert_eq!(user.id, user_id2); + assert_eq!(&user.github_login, "the-new-login2"); + assert_eq!(user.github_user_id, 102); + } +} + #[tokio::test(flavor = "multi_thread")] async fn test_worktree_extensions() { let test_db = TestDb::postgres().await; diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index 94811b0951..f3d43f277e 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -5173,17 +5173,25 @@ impl TestServer { }); let http = FakeHttpClient::with_404_response(); - let user_id = if let Ok(Some(user)) = self.app_state.db.get_user_by_github_login(name).await + let user_id = if let Ok(Some(user)) = self + .app_state + .db + .get_user_by_github_account(name, None) + .await { user.id } else { self.app_state .db - .create_user(&format!("{name}@example.com"), false, NewUserParams { - github_login: name.into(), - github_user_id: 0, - invite_count: 0, - }) + .create_user( + &format!("{name}@example.com"), + false, + NewUserParams { + github_login: name.into(), + github_user_id: 0, + invite_count: 0, + }, + ) .await .unwrap() }; diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 4fc022995f..5f27352c5a 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -1404,7 +1404,7 @@ impl Server { let users = match query.len() { 0 => vec![], 1 | 2 => db - .get_user_by_github_login(&query) + .get_user_by_github_account(&query, None) .await? .into_iter() .collect(), From 758875305b09c03ab47a712c28596feeaf91c0ad Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 20 Sep 2022 16:12:27 -0700 Subject: [PATCH 08/22] Add on delete cascade to signups user_id column --- .../collab/migrations/20220913211150_create_signups.down.sql | 1 - crates/collab/migrations/20220913211150_create_signups.up.sql | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/collab/migrations/20220913211150_create_signups.down.sql b/crates/collab/migrations/20220913211150_create_signups.down.sql index ec02ac3322..59b20b1128 100644 --- a/crates/collab/migrations/20220913211150_create_signups.down.sql +++ b/crates/collab/migrations/20220913211150_create_signups.down.sql @@ -7,4 +7,3 @@ ALTER TABLE users DROP SEQUENCE metrics_id_seq; DROP INDEX index_users_on_email_address; -DROP INDEX index_users_on_github_user_id; \ No newline at end of file diff --git a/crates/collab/migrations/20220913211150_create_signups.up.sql b/crates/collab/migrations/20220913211150_create_signups.up.sql index 3de683c58e..5f02bd6887 100644 --- a/crates/collab/migrations/20220913211150_create_signups.up.sql +++ b/crates/collab/migrations/20220913211150_create_signups.up.sql @@ -7,8 +7,8 @@ CREATE TABLE IF NOT EXISTS "signups" ( "email_confirmation_sent" BOOLEAN NOT NULL, "metrics_id" INTEGER NOT NULL DEFAULT nextval('metrics_id_seq'), "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, - "user_id" INTEGER REFERENCES users (id), - "inviting_user_id" INTEGER REFERENCES users (id), + "user_id" INTEGER REFERENCES users (id) ON DELETE CASCADE, + "inviting_user_id" INTEGER REFERENCES users (id) ON DELETE SET NULL, "platform_mac" BOOLEAN NOT NULL, "platform_linux" BOOLEAN NOT NULL, From 20ec933e23fb2b396e0e00e2589f361737298dbe Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 20 Sep 2022 16:23:02 -0700 Subject: [PATCH 09/22] Proceed gracefully when someone signs up repeatedly --- crates/collab/src/db.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 70dc0c4e5b..517810aa29 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -458,21 +458,29 @@ impl Db for PostgresDb { ) -> Result<(UserId, Option)> { let mut tx = self.pool.begin().await?; - let (signup_id, metrics_id, inviting_user_id): (i32, i32, Option) = sqlx::query_as( + let (signup_id, metrics_id, existing_user_id, inviting_user_id): ( + i32, + i32, + Option, + Option, + ) = sqlx::query_as( " - SELECT id, metrics_id, inviting_user_id + SELECT id, metrics_id, user_id, inviting_user_id FROM signups WHERE email_address = $1 AND - email_confirmation_code = $2 AND - user_id is NULL + email_confirmation_code = $2 ", ) .bind(&invite.email_address) .bind(&invite.email_confirmation_code) .fetch_optional(&mut tx) .await? - .ok_or_else(|| anyhow!("no such invite"))?; + .ok_or_else(|| Error::Http(StatusCode::NOT_FOUND, "no such invite".to_string()))?; + + if existing_user_id.is_some() { + Err(Error::Http(StatusCode::UNPROCESSABLE_ENTITY, "invitation already redeemed".to_string()))?; + } let user_id: UserId = sqlx::query_scalar( " From 7a049f14046ba35253f3a98a61de9c97978e9618 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 21 Sep 2022 10:20:11 -0700 Subject: [PATCH 10/22] Fix error when loading users without github user ids from the db --- crates/collab/src/bin/seed.rs | 25 ++++++++++++++++++------- crates/collab/src/db.rs | 8 ++++---- crates/collab/src/db_tests.rs | 12 ++++++------ 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/crates/collab/src/bin/seed.rs b/crates/collab/src/bin/seed.rs index dba7d14939..b7b3a96710 100644 --- a/crates/collab/src/bin/seed.rs +++ b/crates/collab/src/bin/seed.rs @@ -11,7 +11,7 @@ mod db; #[derive(Debug, Deserialize)] struct GitHubUser { - id: usize, + id: i32, login: String, email: Option, } @@ -26,8 +26,11 @@ async fn main() { let github_token = std::env::var("GITHUB_TOKEN").expect("missing GITHUB_TOKEN env var"); let client = reqwest::Client::new(); - let current_user = + let mut current_user = fetch_github::(&client, &github_token, "https://api.github.com/user").await; + current_user + .email + .get_or_insert_with(|| "placeholder@example.com".to_string()); let staff_users = fetch_github::>( &client, &github_token, @@ -64,16 +67,24 @@ async fn main() { let mut zed_user_ids = Vec::::new(); for (github_user, admin) in zed_users { if let Some(user) = db - .get_user_by_github_login(&github_user.login) + .get_user_by_github_account(&github_user.login, Some(github_user.id)) .await .expect("failed to fetch user") { zed_user_ids.push(user.id); - } else { + } else if let Some(email) = &github_user.email { zed_user_ids.push( - db.create_user(&github_user.login, github_user.email.as_deref(), admin) - .await - .expect("failed to insert user"), + db.create_user( + email, + admin, + db::NewUserParams { + github_login: github_user.login, + github_user_id: github_user.id, + invite_count: 5, + }, + ) + .await + .expect("failed to insert user"), ); } } diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 517810aa29..b4aec0f234 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1563,7 +1563,7 @@ id_type!(UserId); pub struct User { pub id: UserId, pub github_login: String, - pub github_user_id: i32, + pub github_user_id: Option, pub email_address: Option, pub admin: bool, pub invite_code: Option, @@ -1795,7 +1795,7 @@ mod test { User { id: user_id, github_login: params.github_login, - github_user_id: params.github_user_id, + github_user_id: Some(params.github_user_id), email_address: Some(email_address.to_string()), admin, invite_code: None, @@ -1838,12 +1838,12 @@ mod test { self.background.simulate_random_delay().await; if let Some(github_user_id) = github_user_id { for user in self.users.lock().values_mut() { - if user.github_user_id == github_user_id { + if user.github_user_id == Some(github_user_id) { user.github_login = github_login.into(); return Ok(Some(user.clone())); } if user.github_login == github_login { - user.github_user_id = github_user_id; + user.github_user_id = Some(github_user_id); return Ok(Some(user.clone())); } } diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index 49ac053fd8..1d4417dd86 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -69,7 +69,7 @@ async fn test_get_users_by_ids() { User { id: user1, github_login: "u1".to_string(), - github_user_id: 1, + github_user_id: Some(1), email_address: Some("u1@example.com".to_string()), admin: false, ..Default::default() @@ -77,7 +77,7 @@ async fn test_get_users_by_ids() { User { id: user2, github_login: "u2".to_string(), - github_user_id: 2, + github_user_id: Some(2), email_address: Some("u2@example.com".to_string()), admin: false, ..Default::default() @@ -85,7 +85,7 @@ async fn test_get_users_by_ids() { User { id: user3, github_login: "u3".to_string(), - github_user_id: 3, + github_user_id: Some(3), email_address: Some("u3@example.com".to_string()), admin: false, ..Default::default() @@ -93,7 +93,7 @@ async fn test_get_users_by_ids() { User { id: user4, github_login: "u4".to_string(), - github_user_id: 4, + github_user_id: Some(4), email_address: Some("u4@example.com".to_string()), admin: false, ..Default::default() @@ -142,7 +142,7 @@ async fn test_get_user_by_github_account() { .unwrap(); assert_eq!(user.id, user_id1); assert_eq!(&user.github_login, "login1"); - assert_eq!(user.github_user_id, 101); + assert_eq!(user.github_user_id, Some(101)); assert!(db .get_user_by_github_account("non-existent-login", None) @@ -157,7 +157,7 @@ async fn test_get_user_by_github_account() { .unwrap(); assert_eq!(user.id, user_id2); assert_eq!(&user.github_login, "the-new-login2"); - assert_eq!(user.github_user_id, 102); + assert_eq!(user.github_user_id, Some(102)); } } From dac0ce10e59f1f2f11bebbddc33d87d7ab113f01 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 22 Sep 2022 14:37:25 -0700 Subject: [PATCH 11/22] Return the metrics id from the signup-creation API Co-authored-by: Nathan Sobo --- crates/collab/src/api.rs | 11 ++++++++--- crates/collab/src/db.rs | 24 +++++++++++++++--------- crates/collab/src/db_tests.rs | 24 ++++++++++++++---------- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 73293e0b2c..51b43119dc 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -396,12 +396,17 @@ async fn get_user_for_invite_code( Ok(Json(app.db.get_user_for_invite_code(&code).await?)) } +#[derive(Serialize)] +struct CreateSignupResponse { + metrics_id: i32, +} + async fn create_signup( Json(params): Json, Extension(app): Extension>, -) -> Result<()> { - app.db.create_signup(params).await?; - Ok(()) +) -> Result> { + let metrics_id = app.db.create_signup(params).await?; + Ok(Json(CreateSignupResponse { metrics_id })) } async fn get_waitlist_summary( diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index b4aec0f234..81c87f213a 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -37,7 +37,7 @@ pub trait Db: Send + Sync { async fn get_user_for_invite_code(&self, code: &str) -> Result; async fn create_invite_from_code(&self, code: &str, email_address: &str) -> Result; - async fn create_signup(&self, signup: Signup) -> Result<()>; + async fn create_signup(&self, signup: Signup) -> Result; async fn get_waitlist_summary(&self) -> Result; async fn get_unsent_invites(&self, count: usize) -> Result>; async fn record_sent_invites(&self, invites: &[Invite]) -> Result<()>; @@ -364,8 +364,8 @@ impl Db for PostgresDb { // signups - async fn create_signup(&self, signup: Signup) -> Result<()> { - sqlx::query( + async fn create_signup(&self, signup: Signup) -> Result { + Ok(sqlx::query_scalar( " INSERT INTO signups ( @@ -381,6 +381,7 @@ impl Db for PostgresDb { ) VALUES ($1, $2, 'f', $3, $4, $5, 'f', $6, $7) + RETURNING id ", ) .bind(&signup.email_address) @@ -390,9 +391,8 @@ impl Db for PostgresDb { .bind(&signup.platform_windows) .bind(&signup.editor_features) .bind(&signup.programming_languages) - .execute(&self.pool) - .await?; - Ok(()) + .fetch_one(&self.pool) + .await?) } async fn get_waitlist_summary(&self) -> Result { @@ -479,7 +479,10 @@ impl Db for PostgresDb { .ok_or_else(|| Error::Http(StatusCode::NOT_FOUND, "no such invite".to_string()))?; if existing_user_id.is_some() { - Err(Error::Http(StatusCode::UNPROCESSABLE_ENTITY, "invitation already redeemed".to_string()))?; + Err(Error::Http( + StatusCode::UNPROCESSABLE_ENTITY, + "invitation already redeemed".to_string(), + ))?; } let user_id: UserId = sqlx::query_scalar( @@ -1564,6 +1567,7 @@ pub struct User { pub id: UserId, pub github_login: String, pub github_user_id: Option, + pub metrics_id: i32, pub email_address: Option, pub admin: bool, pub invite_code: Option, @@ -1789,7 +1793,8 @@ mod test { { Ok(user.id) } else { - let user_id = UserId(post_inc(&mut *self.next_user_id.lock())); + let id = post_inc(&mut *self.next_user_id.lock()); + let user_id = UserId(id); users.insert( user_id, User { @@ -1797,6 +1802,7 @@ mod test { github_login: params.github_login, github_user_id: Some(params.github_user_id), email_address: Some(email_address.to_string()), + metrics_id: id + 100, admin, invite_code: None, invite_count: 0, @@ -1878,7 +1884,7 @@ mod test { // signups - async fn create_signup(&self, _signup: Signup) -> Result<()> { + async fn create_signup(&self, _signup: Signup) -> Result { unimplemented!() } diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index 1d4417dd86..64a3626a22 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -1139,17 +1139,20 @@ async fn test_signups() { let db = postgres.db(); // people sign up on the waitlist + let mut signup_metric_ids = Vec::new(); for i in 0..8 { - db.create_signup(Signup { - email_address: format!("person-{i}@example.com"), - platform_mac: true, - platform_linux: i % 2 == 0, - platform_windows: i % 4 == 0, - editor_features: vec!["speed".into()], - programming_languages: vec!["rust".into(), "c".into()], - }) - .await - .unwrap(); + signup_metric_ids.push( + db.create_signup(Signup { + email_address: format!("person-{i}@example.com"), + platform_mac: true, + platform_linux: i % 2 == 0, + platform_windows: i % 4 == 0, + editor_features: vec!["speed".into()], + programming_languages: vec!["rust".into(), "c".into()], + }) + .await + .unwrap(), + ); } assert_eq!( @@ -1235,6 +1238,7 @@ async fn test_signups() { assert_eq!(user.github_login, "person-0"); assert_eq!(user.email_address.as_deref(), Some("person-0@example.com")); assert_eq!(user.invite_count, 5); + assert_eq!(user.metrics_id, signup_metric_ids[0]); // cannot redeem the same signup again. db.create_user_from_invite( From 04baccbea6a002f62e361020215acd7d82d21a01 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 22 Sep 2022 17:52:39 -0700 Subject: [PATCH 12/22] Start work on a client-side telemetry system --- Cargo.lock | 1 + crates/client/Cargo.toml | 1 + crates/client/src/channel.rs | 2 +- crates/client/src/client.rs | 31 +++-- crates/client/src/telemetry.rs | 128 ++++++++++++++++++++ crates/collab/src/integration_tests.rs | 2 +- crates/contacts_panel/src/contacts_panel.rs | 2 +- crates/gpui/src/platform.rs | 1 + crates/gpui/src/platform/mac/platform.rs | 17 ++- crates/gpui/src/platform/test.rs | 8 ++ crates/project/src/project.rs | 2 +- crates/project/src/worktree.rs | 14 +-- crates/workspace/src/workspace.rs | 2 +- crates/zed/src/main.rs | 10 +- 14 files changed, 191 insertions(+), 30 deletions(-) create mode 100644 crates/client/src/telemetry.rs diff --git a/Cargo.lock b/Cargo.lock index 05701b7c56..d11d776732 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -955,6 +955,7 @@ dependencies = [ "postage", "rand 0.8.5", "rpc", + "serde", "smol", "sum_tree", "thiserror", diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index a7888b8965..5fcff565bb 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -32,6 +32,7 @@ thiserror = "1.0.29" time = { version = "0.3", features = ["serde", "serde-well-known"] } tiny_http = "0.8" url = "2.2" +serde = { version = "*", features = ["derive"] } [dev-dependencies] collections = { path = "../collections", features = ["test-support"] } diff --git a/crates/client/src/channel.rs b/crates/client/src/channel.rs index a88f872d11..3f99d7ccd2 100644 --- a/crates/client/src/channel.rs +++ b/crates/client/src/channel.rs @@ -601,7 +601,7 @@ mod tests { let user_id = 5; let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); + let client = cx.update(|cx| Client::new(http_client.clone(), cx)); let server = FakeServer::for_client(user_id, &client, cx).await; Channel::init(&client); diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index e328108a52..b5e4558155 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -3,6 +3,7 @@ pub mod test; pub mod channel; pub mod http; +pub mod telemetry; pub mod user; use anyhow::{anyhow, Context, Result}; @@ -13,8 +14,9 @@ use async_tungstenite::tungstenite::{ }; use futures::{future::LocalBoxFuture, FutureExt, SinkExt, StreamExt, TryStreamExt}; use gpui::{ - actions, AnyModelHandle, AnyViewHandle, AnyWeakModelHandle, AnyWeakViewHandle, AsyncAppContext, - Entity, ModelContext, ModelHandle, MutableAppContext, Task, View, ViewContext, ViewHandle, + actions, serde_json::Value, AnyModelHandle, AnyViewHandle, AnyWeakModelHandle, + AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, + MutableAppContext, Task, View, ViewContext, ViewHandle, }; use http::HttpClient; use lazy_static::lazy_static; @@ -31,6 +33,7 @@ use std::{ sync::{Arc, Weak}, time::{Duration, Instant}, }; +use telemetry::Telemetry; use thiserror::Error; use url::Url; use util::{ResultExt, TryFutureExt}; @@ -63,6 +66,7 @@ pub struct Client { id: usize, peer: Arc, http: Arc, + telemetry: Arc, state: RwLock, #[allow(clippy::type_complexity)] @@ -232,10 +236,11 @@ impl Drop for Subscription { } impl Client { - pub fn new(http: Arc) -> Arc { + pub fn new(http: Arc, cx: &AppContext) -> Arc { Arc::new(Self { id: 0, peer: Peer::new(), + telemetry: Telemetry::new(http.clone(), cx), http, state: Default::default(), @@ -595,6 +600,9 @@ impl Client { if credentials.is_none() && try_keychain { credentials = read_credentials_from_keychain(cx); read_from_keychain = credentials.is_some(); + if read_from_keychain { + self.log_event("read_credentials_from_keychain", Default::default()); + } } if credentials.is_none() { let mut status_rx = self.status(); @@ -878,6 +886,7 @@ impl Client { ) -> Task> { let platform = cx.platform(); let executor = cx.background(); + let telemetry = self.telemetry.clone(); executor.clone().spawn(async move { // Generate a pair of asymmetric encryption keys. The public key will be used by the // zed server to encrypt the user's access token, so that it can'be intercepted by @@ -956,6 +965,8 @@ impl Client { .context("failed to decrypt access token")?; platform.activate(true); + telemetry.log_event("authenticate_with_browser", Default::default()); + Ok(Credentials { user_id: user_id.parse()?, access_token, @@ -1020,6 +1031,10 @@ impl Client { log::debug!("rpc respond. client_id:{}. name:{}", self.id, T::NAME); self.peer.respond_with_error(receipt, error) } + + pub fn log_event(&self, kind: &str, properties: Value) { + self.telemetry.log_event(kind, properties) + } } impl AnyWeakEntityHandle { @@ -1085,7 +1100,7 @@ mod tests { cx.foreground().forbid_parking(); let user_id = 5; - let client = Client::new(FakeHttpClient::with_404_response()); + let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let server = FakeServer::for_client(user_id, &client, cx).await; let mut status = client.status(); assert!(matches!( @@ -1124,7 +1139,7 @@ mod tests { let auth_count = Arc::new(Mutex::new(0)); let dropped_auth_count = Arc::new(Mutex::new(0)); - let client = Client::new(FakeHttpClient::with_404_response()); + let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); client.override_authenticate({ let auth_count = auth_count.clone(); let dropped_auth_count = dropped_auth_count.clone(); @@ -1173,7 +1188,7 @@ mod tests { cx.foreground().forbid_parking(); let user_id = 5; - let client = Client::new(FakeHttpClient::with_404_response()); + let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let server = FakeServer::for_client(user_id, &client, cx).await; let (done_tx1, mut done_rx1) = smol::channel::unbounded(); @@ -1219,7 +1234,7 @@ mod tests { cx.foreground().forbid_parking(); let user_id = 5; - let client = Client::new(FakeHttpClient::with_404_response()); + let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let server = FakeServer::for_client(user_id, &client, cx).await; let model = cx.add_model(|_| Model::default()); @@ -1247,7 +1262,7 @@ mod tests { cx.foreground().forbid_parking(); let user_id = 5; - let client = Client::new(FakeHttpClient::with_404_response()); + let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let server = FakeServer::for_client(user_id, &client, cx).await; let model = cx.add_model(|_| Model::default()); diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs new file mode 100644 index 0000000000..a96dd26c20 --- /dev/null +++ b/crates/client/src/telemetry.rs @@ -0,0 +1,128 @@ +use crate::{http::HttpClient, ZED_SECRET_CLIENT_TOKEN}; +use gpui::{ + executor::Background, + serde_json::{self, value::Map, Value}, + AppContext, AppVersion, Task, +}; +use isahc::Request; +use parking_lot::Mutex; +use serde::Serialize; +use std::{ + mem, + sync::Arc, + time::{Duration, SystemTime, UNIX_EPOCH}, +}; +use util::ResultExt; + +pub struct Telemetry { + client: Arc, + executor: Arc, + state: Mutex, +} + +#[derive(Default)] +struct TelemetryState { + metrics_id: Option, + device_id: Option, + app_version: Option, + os_version: Option, + queue: Vec, + flush_task: Option>, +} + +#[derive(Serialize)] +struct RecordEventParams { + token: &'static str, + metrics_id: Option, + device_id: Option, + app_version: Option, + os_version: Option, + events: Vec, +} + +#[derive(Serialize)] +struct Event { + #[serde(rename = "type")] + kind: String, + time: u128, + properties: Option>, +} + +const MAX_QUEUE_LEN: usize = 30; +const EVENTS_URI: &'static str = "https://zed.dev/api/telemetry"; +const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(30); + +impl Telemetry { + pub fn new(client: Arc, cx: &AppContext) -> Arc { + let platform = cx.platform(); + Arc::new(Self { + client, + executor: cx.background().clone(), + state: Mutex::new(TelemetryState { + os_version: platform.os_version().log_err(), + app_version: platform.app_version().log_err(), + metrics_id: None, + device_id: None, + queue: Default::default(), + flush_task: Default::default(), + }), + }) + } + + pub fn set_metrics_id(&self, metrics_id: Option) { + self.state.lock().metrics_id = metrics_id; + } + + pub fn log_event(self: &Arc, kind: &str, properties: Value) { + let mut state = self.state.lock(); + state.queue.push(Event { + kind: kind.to_string(), + time: SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_millis(), + properties: if let Value::Object(properties) = properties { + Some(properties) + } else { + None + }, + }); + if state.queue.len() >= MAX_QUEUE_LEN { + self.flush(); + } else { + let this = self.clone(); + let executor = self.executor.clone(); + state.flush_task = Some(self.executor.spawn(async move { + executor.timer(DEBOUNCE_INTERVAL).await; + this.flush(); + })); + } + } + + fn flush(&self) { + let mut state = self.state.lock(); + let events = mem::take(&mut state.queue); + let client = self.client.clone(); + let app_version = state.app_version; + let os_version = state.os_version; + let metrics_id = state.metrics_id; + let device_id = state.device_id.clone(); + state.flush_task.take(); + self.executor + .spawn(async move { + let body = serde_json::to_vec(&RecordEventParams { + token: ZED_SECRET_CLIENT_TOKEN, + events, + app_version: app_version.map(|v| v.to_string()), + os_version: os_version.map(|v| v.to_string()), + metrics_id, + device_id, + }) + .log_err()?; + let request = Request::post(EVENTS_URI).body(body.into()).log_err()?; + client.send(request).await.log_err(); + Some(()) + }) + .detach(); + } +} diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index f3d43f277e..2b9dd25a90 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -5196,7 +5196,7 @@ impl TestServer { .unwrap() }; let client_name = name.to_string(); - let mut client = Client::new(http.clone()); + let mut client = cx.read(|cx| Client::new(http.clone(), cx)); let server = self.server.clone(); let db = self.app_state.db.clone(); let connection_killers = self.connection_killers.clone(); diff --git a/crates/contacts_panel/src/contacts_panel.rs b/crates/contacts_panel/src/contacts_panel.rs index b5460f4d06..7dcfb8cea4 100644 --- a/crates/contacts_panel/src/contacts_panel.rs +++ b/crates/contacts_panel/src/contacts_panel.rs @@ -1216,7 +1216,7 @@ mod tests { let languages = Arc::new(LanguageRegistry::test()); let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); + let client = cx.read(|cx| Client::new(http_client.clone(), cx)); let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); let project_store = cx.add_model(|_| ProjectStore::new(project::Db::open_fake())); let server = FakeServer::for_client(current_user_id, &client, cx).await; diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index a50698070c..7467dad547 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -69,6 +69,7 @@ pub trait Platform: Send + Sync { fn path_for_auxiliary_executable(&self, name: &str) -> Result; fn app_path(&self) -> Result; fn app_version(&self) -> Result; + fn os_version(&self) -> Result; } pub(crate) trait ForegroundPlatform { diff --git a/crates/gpui/src/platform/mac/platform.rs b/crates/gpui/src/platform/mac/platform.rs index 7732da2b3e..02fe73504e 100644 --- a/crates/gpui/src/platform/mac/platform.rs +++ b/crates/gpui/src/platform/mac/platform.rs @@ -4,7 +4,7 @@ use super::{ use crate::{ executor, keymap, platform::{self, CursorStyle}, - Action, ClipboardItem, Event, Menu, MenuItem, + Action, AppVersion, ClipboardItem, Event, Menu, MenuItem, }; use anyhow::{anyhow, Result}; use block::ConcreteBlock; @@ -16,7 +16,8 @@ use cocoa::{ }, base::{id, nil, selector, YES}, foundation::{ - NSArray, NSAutoreleasePool, NSBundle, NSData, NSInteger, NSString, NSUInteger, NSURL, + NSArray, NSAutoreleasePool, NSBundle, NSData, NSInteger, NSProcessInfo, NSString, + NSUInteger, NSURL, }, }; use core_foundation::{ @@ -748,6 +749,18 @@ impl platform::Platform for MacPlatform { } } } + + fn os_version(&self) -> Result { + unsafe { + let process_info = NSProcessInfo::processInfo(nil); + let version = process_info.operatingSystemVersion(); + Ok(AppVersion { + major: version.majorVersion as usize, + minor: version.minorVersion as usize, + patch: version.patchVersion as usize, + }) + } + } } unsafe fn path_from_objc(path: id) -> PathBuf { diff --git a/crates/gpui/src/platform/test.rs b/crates/gpui/src/platform/test.rs index 9a458a1dd9..1bb53d49ab 100644 --- a/crates/gpui/src/platform/test.rs +++ b/crates/gpui/src/platform/test.rs @@ -196,6 +196,14 @@ impl super::Platform for Platform { patch: 0, }) } + + fn os_version(&self) -> Result { + Ok(AppVersion { + major: 1, + minor: 0, + patch: 0, + }) + } } impl Window { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 09c5a72315..abb55e49b0 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -650,7 +650,7 @@ impl Project { let languages = Arc::new(LanguageRegistry::test()); let http_client = client::test::FakeHttpClient::with_404_response(); - let client = client::Client::new(http_client.clone()); + let client = cx.update(|cx| client::Client::new(http_client.clone(), cx)); let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); let project_store = cx.add_model(|_| ProjectStore::new(Db::open_fake())); let project = cx.update(|cx| { diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 96ebb59de0..74c50e0c5f 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -2804,7 +2804,7 @@ mod tests { .await; let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client); + let client = cx.read(|cx| Client::new(http_client, cx)); let tree = Worktree::local( client, @@ -2866,8 +2866,7 @@ mod tests { fs.insert_symlink("/root/lib/a/lib", "..".into()).await; fs.insert_symlink("/root/lib/b/lib", "..".into()).await; - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client); + let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let tree = Worktree::local( client, Arc::from(Path::new("/root")), @@ -2945,8 +2944,7 @@ mod tests { })); let dir = parent_dir.path().join("tree"); - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); + let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let tree = Worktree::local( client, @@ -3016,8 +3014,7 @@ mod tests { "ignored-dir": {} })); - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); + let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let tree = Worktree::local( client, @@ -3064,8 +3061,7 @@ mod tests { #[gpui::test(iterations = 30)] async fn test_create_directory(cx: &mut TestAppContext) { - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); + let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let fs = FakeFs::new(cx.background()); fs.insert_tree( diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 017964d9a1..b9cface656 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -856,7 +856,7 @@ impl AppState { let fs = project::FakeFs::new(cx.background().clone()); let languages = Arc::new(LanguageRegistry::test()); let http_client = client::test::FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); + let client = Client::new(http_client.clone(), cx); let project_store = cx.add_model(|_| ProjectStore::new(project::Db::open_fake())); let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); let themes = ThemeRegistry::new((), cx.font_cache().clone()); diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 3bfd5e6e1a..bb913ab610 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -20,7 +20,7 @@ use futures::{ FutureExt, SinkExt, StreamExt, }; use gpui::{executor::Background, App, AssetSource, AsyncAppContext, Task, ViewContext}; -use isahc::{config::Configurable, AsyncBody, Request}; +use isahc::{config::Configurable, Request}; use language::LanguageRegistry; use log::LevelFilter; use parking_lot::Mutex; @@ -88,7 +88,7 @@ fn main() { }); app.run(move |cx| { - let client = client::Client::new(http.clone()); + let client = client::Client::new(http.clone(), cx); let mut languages = LanguageRegistry::new(login_shell_env_loaded); languages.set_language_server_download_dir(zed::paths::LANGUAGES_DIR.clone()); let languages = Arc::new(languages); @@ -280,12 +280,10 @@ fn init_panic_hook(app_version: String, http: Arc, background: A "token": ZED_SECRET_CLIENT_TOKEN, })) .unwrap(); - let request = Request::builder() - .uri(&panic_report_url) - .method(http::Method::POST) + let request = Request::post(&panic_report_url) .redirect_policy(isahc::config::RedirectPolicy::Follow) .header("Content-Type", "application/json") - .body(AsyncBody::from(body))?; + .body(body.into())?; let response = http.send(request).await.context("error sending panic")?; if response.status().is_success() { fs::remove_file(child_path) From 4784dbe498236d3333453d87d3822c7cb965510b Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2022 17:06:27 -0700 Subject: [PATCH 13/22] Link signups to users in telemetry via a stored device_id Co-authored-by: Joseph Lyons --- crates/client/src/client.rs | 34 ++++++-- crates/client/src/telemetry.rs | 26 +++--- .../20220913211150_create_signups.down.sql | 6 +- .../20220913211150_create_signups.up.sql | 12 +-- crates/collab/src/api.rs | 80 ++++++++++--------- crates/collab/src/db.rs | 41 +++++----- crates/collab/src/db_tests.rs | 34 ++++---- 7 files changed, 124 insertions(+), 109 deletions(-) diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index b5e4558155..3d85aea3c5 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -14,9 +14,11 @@ use async_tungstenite::tungstenite::{ }; use futures::{future::LocalBoxFuture, FutureExt, SinkExt, StreamExt, TryStreamExt}; use gpui::{ - actions, serde_json::Value, AnyModelHandle, AnyViewHandle, AnyWeakModelHandle, - AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, - MutableAppContext, Task, View, ViewContext, ViewHandle, + actions, + serde_json::{json, Value}, + AnyModelHandle, AnyViewHandle, AnyWeakModelHandle, AnyWeakViewHandle, AppContext, + AsyncAppContext, Entity, ModelContext, ModelHandle, MutableAppContext, Task, View, ViewContext, + ViewHandle, }; use http::HttpClient; use lazy_static::lazy_static; @@ -52,13 +54,29 @@ lazy_static! { pub const ZED_SECRET_CLIENT_TOKEN: &str = "618033988749894"; -actions!(client, [Authenticate]); +actions!(client, [Authenticate, TestTelemetry]); -pub fn init(rpc: Arc, cx: &mut MutableAppContext) { - cx.add_global_action(move |_: &Authenticate, cx| { - let rpc = rpc.clone(); - cx.spawn(|cx| async move { rpc.authenticate_and_connect(true, &cx).log_err().await }) +pub fn init(client: Arc, cx: &mut MutableAppContext) { + cx.add_global_action({ + let client = client.clone(); + move |_: &Authenticate, cx| { + let client = client.clone(); + cx.spawn( + |cx| async move { client.authenticate_and_connect(true, &cx).log_err().await }, + ) .detach(); + } + }); + cx.add_global_action({ + let client = client.clone(); + move |_: &TestTelemetry, _| { + client.log_event( + "test_telemetry", + json!({ + "test_property": "test_value" + }), + ) + } }); } diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index a96dd26c20..a78e691459 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -1,4 +1,4 @@ -use crate::{http::HttpClient, ZED_SECRET_CLIENT_TOKEN}; +use crate::{http::HttpClient, ZED_SECRET_CLIENT_TOKEN, ZED_SERVER_URL}; use gpui::{ executor::Background, serde_json::{self, value::Map, Value}, @@ -22,7 +22,6 @@ pub struct Telemetry { #[derive(Default)] struct TelemetryState { - metrics_id: Option, device_id: Option, app_version: Option, os_version: Option, @@ -33,7 +32,6 @@ struct TelemetryState { #[derive(Serialize)] struct RecordEventParams { token: &'static str, - metrics_id: Option, device_id: Option, app_version: Option, os_version: Option, @@ -48,8 +46,13 @@ struct Event { properties: Option>, } -const MAX_QUEUE_LEN: usize = 30; -const EVENTS_URI: &'static str = "https://zed.dev/api/telemetry"; +#[cfg(debug_assertions)] +const MAX_QUEUE_LEN: usize = 1; + +#[cfg(not(debug_assertions))] +const MAX_QUEUE_LEN: usize = 10; + +const EVENTS_URI: &'static str = "api/telemetry"; const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(30); impl Telemetry { @@ -61,7 +64,6 @@ impl Telemetry { state: Mutex::new(TelemetryState { os_version: platform.os_version().log_err(), app_version: platform.app_version().log_err(), - metrics_id: None, device_id: None, queue: Default::default(), flush_task: Default::default(), @@ -69,10 +71,6 @@ impl Telemetry { }) } - pub fn set_metrics_id(&self, metrics_id: Option) { - self.state.lock().metrics_id = metrics_id; - } - pub fn log_event(self: &Arc, kind: &str, properties: Value) { let mut state = self.state.lock(); state.queue.push(Event { @@ -88,6 +86,7 @@ impl Telemetry { }, }); if state.queue.len() >= MAX_QUEUE_LEN { + drop(state); self.flush(); } else { let this = self.clone(); @@ -105,7 +104,6 @@ impl Telemetry { let client = self.client.clone(); let app_version = state.app_version; let os_version = state.os_version; - let metrics_id = state.metrics_id; let device_id = state.device_id.clone(); state.flush_task.take(); self.executor @@ -115,11 +113,13 @@ impl Telemetry { events, app_version: app_version.map(|v| v.to_string()), os_version: os_version.map(|v| v.to_string()), - metrics_id, device_id, }) .log_err()?; - let request = Request::post(EVENTS_URI).body(body.into()).log_err()?; + let request = Request::post(format!("{}/{}", *ZED_SERVER_URL, EVENTS_URI)) + .header("Content-Type", "application/json") + .body(body.into()) + .log_err()?; client.send(request).await.log_err(); Some(()) }) diff --git a/crates/collab/migrations/20220913211150_create_signups.down.sql b/crates/collab/migrations/20220913211150_create_signups.down.sql index 59b20b1128..f67c10dd01 100644 --- a/crates/collab/migrations/20220913211150_create_signups.down.sql +++ b/crates/collab/migrations/20220913211150_create_signups.down.sql @@ -1,9 +1,7 @@ DROP TABLE signups; ALTER TABLE users - DROP COLUMN github_user_id, - DROP COLUMN metrics_id; - -DROP SEQUENCE metrics_id_seq; + DROP COLUMN github_user_id; DROP INDEX index_users_on_email_address; +DROP INDEX index_users_on_github_user_id; diff --git a/crates/collab/migrations/20220913211150_create_signups.up.sql b/crates/collab/migrations/20220913211150_create_signups.up.sql index 5f02bd6887..35e334ea5f 100644 --- a/crates/collab/migrations/20220913211150_create_signups.up.sql +++ b/crates/collab/migrations/20220913211150_create_signups.up.sql @@ -1,12 +1,10 @@ -CREATE SEQUENCE metrics_id_seq; - CREATE TABLE IF NOT EXISTS "signups" ( - "id" SERIAL PRIMARY KEY NOT NULL, + "id" SERIAL PRIMARY KEY, "email_address" VARCHAR NOT NULL, "email_confirmation_code" VARCHAR(64) NOT NULL, "email_confirmation_sent" BOOLEAN NOT NULL, - "metrics_id" INTEGER NOT NULL DEFAULT nextval('metrics_id_seq'), "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + "device_id" VARCHAR NOT NULL, "user_id" INTEGER REFERENCES users (id) ON DELETE CASCADE, "inviting_user_id" INTEGER REFERENCES users (id) ON DELETE SET NULL, @@ -23,11 +21,7 @@ CREATE UNIQUE INDEX "index_signups_on_email_address" ON "signups" ("email_addres CREATE INDEX "index_signups_on_email_confirmation_sent" ON "signups" ("email_confirmation_sent"); ALTER TABLE "users" - ADD "github_user_id" INTEGER, - ADD "metrics_id" INTEGER DEFAULT nextval('metrics_id_seq'); + ADD "github_user_id" INTEGER; CREATE INDEX "index_users_on_email_address" ON "users" ("email_address"); CREATE INDEX "index_users_on_github_user_id" ON "users" ("github_user_id"); - -UPDATE users -SET metrics_id = nextval('metrics_id_seq'); diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 51b43119dc..a82363a56b 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -127,44 +127,52 @@ struct CreateUserParams { invite_count: i32, } +#[derive(Serialize, Debug)] +struct CreateUserResponse { + user: User, + signup_device_id: Option, +} + async fn create_user( Json(params): Json, Extension(app): Extension>, Extension(rpc_server): Extension>, -) -> Result> { +) -> Result> { let user = NewUserParams { github_login: params.github_login, github_user_id: params.github_user_id, invite_count: params.invite_count, }; - let (user_id, inviter_id) = - // Creating a user via the normal signup process - if let Some(email_confirmation_code) = params.email_confirmation_code { - app.db - .create_user_from_invite( - &Invite { - email_address: params.email_address, - email_confirmation_code, - }, - user, - ) - .await? - } - // Creating a user as an admin - else { - ( - app.db - .create_user(¶ms.email_address, false, user) - .await?, - None, + let user_id; + let signup_device_id; + // Creating a user via the normal signup process + if let Some(email_confirmation_code) = params.email_confirmation_code { + let result = app + .db + .create_user_from_invite( + &Invite { + email_address: params.email_address, + email_confirmation_code, + }, + user, ) - }; - - if let Some(inviter_id) = inviter_id { - rpc_server - .invite_code_redeemed(inviter_id, user_id) - .await - .trace_err(); + .await?; + user_id = result.0; + signup_device_id = Some(result.2); + if let Some(inviter_id) = result.1 { + rpc_server + .invite_code_redeemed(inviter_id, user_id) + .await + .trace_err(); + } + } + // Creating a user as an admin + else { + user_id = app + .db + .create_user(¶ms.email_address, false, user) + .await?; + signup_device_id = None; } let user = app @@ -173,7 +181,10 @@ async fn create_user( .await? .ok_or_else(|| anyhow!("couldn't find the user we just created"))?; - Ok(Json(user)) + Ok(Json(CreateUserResponse { + user, + signup_device_id, + })) } #[derive(Deserialize)] @@ -396,17 +407,12 @@ async fn get_user_for_invite_code( Ok(Json(app.db.get_user_for_invite_code(&code).await?)) } -#[derive(Serialize)] -struct CreateSignupResponse { - metrics_id: i32, -} - async fn create_signup( Json(params): Json, Extension(app): Extension>, -) -> Result> { - let metrics_id = app.db.create_signup(params).await?; - Ok(Json(CreateSignupResponse { metrics_id })) +) -> Result<()> { + app.db.create_signup(params).await?; + Ok(()) } async fn get_waitlist_summary( diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 81c87f213a..1518ec179f 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -37,7 +37,7 @@ pub trait Db: Send + Sync { async fn get_user_for_invite_code(&self, code: &str) -> Result; async fn create_invite_from_code(&self, code: &str, email_address: &str) -> Result; - async fn create_signup(&self, signup: Signup) -> Result; + async fn create_signup(&self, signup: Signup) -> Result<()>; async fn get_waitlist_summary(&self) -> Result; async fn get_unsent_invites(&self, count: usize) -> Result>; async fn record_sent_invites(&self, invites: &[Invite]) -> Result<()>; @@ -45,7 +45,7 @@ pub trait Db: Send + Sync { &self, invite: &Invite, user: NewUserParams, - ) -> Result<(UserId, Option)>; + ) -> Result<(UserId, Option, String)>; /// Registers a new project for the given user. async fn register_project(&self, host_user_id: UserId) -> Result; @@ -364,8 +364,8 @@ impl Db for PostgresDb { // signups - async fn create_signup(&self, signup: Signup) -> Result { - Ok(sqlx::query_scalar( + async fn create_signup(&self, signup: Signup) -> Result<()> { + sqlx::query( " INSERT INTO signups ( @@ -377,10 +377,11 @@ impl Db for PostgresDb { platform_windows, platform_unknown, editor_features, - programming_languages + programming_languages, + device_id ) VALUES - ($1, $2, 'f', $3, $4, $5, 'f', $6, $7) + ($1, $2, 'f', $3, $4, $5, 'f', $6, $7, $8) RETURNING id ", ) @@ -391,8 +392,10 @@ impl Db for PostgresDb { .bind(&signup.platform_windows) .bind(&signup.editor_features) .bind(&signup.programming_languages) - .fetch_one(&self.pool) - .await?) + .bind(&signup.device_id) + .execute(&self.pool) + .await?; + Ok(()) } async fn get_waitlist_summary(&self) -> Result { @@ -455,17 +458,17 @@ impl Db for PostgresDb { &self, invite: &Invite, user: NewUserParams, - ) -> Result<(UserId, Option)> { + ) -> Result<(UserId, Option, String)> { let mut tx = self.pool.begin().await?; - let (signup_id, metrics_id, existing_user_id, inviting_user_id): ( - i32, + let (signup_id, existing_user_id, inviting_user_id, device_id): ( i32, Option, Option, + String, ) = sqlx::query_as( " - SELECT id, metrics_id, user_id, inviting_user_id + SELECT id, user_id, inviting_user_id, device_id FROM signups WHERE email_address = $1 AND @@ -488,9 +491,9 @@ impl Db for PostgresDb { let user_id: UserId = sqlx::query_scalar( " INSERT INTO users - (email_address, github_login, github_user_id, admin, invite_count, invite_code, metrics_id) + (email_address, github_login, github_user_id, admin, invite_count, invite_code) VALUES - ($1, $2, $3, 'f', $4, $5, $6) + ($1, $2, $3, 'f', $4, $5) RETURNING id ", ) @@ -499,7 +502,6 @@ impl Db for PostgresDb { .bind(&user.github_user_id) .bind(&user.invite_count) .bind(random_invite_code()) - .bind(metrics_id) .fetch_one(&mut tx) .await?; @@ -550,7 +552,7 @@ impl Db for PostgresDb { } tx.commit().await?; - Ok((user_id, inviting_user_id)) + Ok((user_id, inviting_user_id, device_id)) } // invite codes @@ -1567,7 +1569,6 @@ pub struct User { pub id: UserId, pub github_login: String, pub github_user_id: Option, - pub metrics_id: i32, pub email_address: Option, pub admin: bool, pub invite_code: Option, @@ -1674,6 +1675,7 @@ pub struct Signup { pub platform_linux: bool, pub editor_features: Vec, pub programming_languages: Vec, + pub device_id: String, } #[derive(Clone, Debug, PartialEq, Deserialize, Serialize, FromRow)] @@ -1802,7 +1804,6 @@ mod test { github_login: params.github_login, github_user_id: Some(params.github_user_id), email_address: Some(email_address.to_string()), - metrics_id: id + 100, admin, invite_code: None, invite_count: 0, @@ -1884,7 +1885,7 @@ mod test { // signups - async fn create_signup(&self, _signup: Signup) -> Result { + async fn create_signup(&self, _signup: Signup) -> Result<()> { unimplemented!() } @@ -1904,7 +1905,7 @@ mod test { &self, _invite: &Invite, _user: NewUserParams, - ) -> Result<(UserId, Option)> { + ) -> Result<(UserId, Option, String)> { unimplemented!() } diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index 64a3626a22..44697a59bd 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -957,7 +957,7 @@ async fn test_invite_codes() { .create_invite_from_code(&invite_code, "u2@example.com") .await .unwrap(); - let (user2, inviter) = db + let (user2, inviter, _) = db .create_user_from_invite( &user2_invite, NewUserParams { @@ -1007,7 +1007,7 @@ async fn test_invite_codes() { .create_invite_from_code(&invite_code, "u3@example.com") .await .unwrap(); - let (user3, inviter) = db + let (user3, inviter, _) = db .create_user_from_invite( &user3_invite, NewUserParams { @@ -1072,7 +1072,7 @@ async fn test_invite_codes() { .create_invite_from_code(&invite_code, "u4@example.com") .await .unwrap(); - let (user4, _) = db + let (user4, _, _) = db .create_user_from_invite( &user4_invite, NewUserParams { @@ -1139,20 +1139,18 @@ async fn test_signups() { let db = postgres.db(); // people sign up on the waitlist - let mut signup_metric_ids = Vec::new(); for i in 0..8 { - signup_metric_ids.push( - db.create_signup(Signup { - email_address: format!("person-{i}@example.com"), - platform_mac: true, - platform_linux: i % 2 == 0, - platform_windows: i % 4 == 0, - editor_features: vec!["speed".into()], - programming_languages: vec!["rust".into(), "c".into()], - }) - .await - .unwrap(), - ); + db.create_signup(Signup { + email_address: format!("person-{i}@example.com"), + platform_mac: true, + platform_linux: i % 2 == 0, + platform_windows: i % 4 == 0, + editor_features: vec!["speed".into()], + programming_languages: vec!["rust".into(), "c".into()], + device_id: format!("device_id_{i}"), + }) + .await + .unwrap(); } assert_eq!( @@ -1219,7 +1217,7 @@ async fn test_signups() { // user completes the signup process by providing their // github account. - let (user_id, inviter_id) = db + let (user_id, inviter_id, signup_device_id) = db .create_user_from_invite( &Invite { email_address: signups_batch1[0].email_address.clone(), @@ -1238,7 +1236,7 @@ async fn test_signups() { assert_eq!(user.github_login, "person-0"); assert_eq!(user.email_address.as_deref(), Some("person-0@example.com")); assert_eq!(user.invite_count, 5); - assert_eq!(user.metrics_id, signup_metric_ids[0]); + assert_eq!(signup_device_id, "device_id_0"); // cannot redeem the same signup again. db.create_user_from_invite( From da36eb3b41b5737fbdb852059fdb14e6a84ebd57 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 26 Sep 2022 15:23:10 -0700 Subject: [PATCH 14/22] wip --- crates/client/src/telemetry.rs | 91 +++++++++++++++--------- crates/gpui/src/platform.rs | 1 + crates/gpui/src/platform/mac/platform.rs | 4 ++ crates/gpui/src/platform/test.rs | 4 ++ 4 files changed, 67 insertions(+), 33 deletions(-) diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index a78e691459..7eea13a923 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -1,8 +1,8 @@ -use crate::{http::HttpClient, ZED_SECRET_CLIENT_TOKEN, ZED_SERVER_URL}; +use crate::{http::HttpClient, ZED_SECRET_CLIENT_TOKEN}; use gpui::{ executor::Background, serde_json::{self, value::Map, Value}, - AppContext, AppVersion, Task, + AppContext, Task, }; use isahc::Request; use parking_lot::Mutex; @@ -12,38 +12,48 @@ use std::{ sync::Arc, time::{Duration, SystemTime, UNIX_EPOCH}, }; -use util::ResultExt; +use util::{post_inc, ResultExt}; pub struct Telemetry { client: Arc, executor: Arc, + session_id: u128, state: Mutex, } #[derive(Default)] struct TelemetryState { - device_id: Option, - app_version: Option, - os_version: Option, - queue: Vec, + user_id: Option>, + device_id: Option>, + app_version: Option>, + os_version: Option>, + os_name: &'static str, + queue: Vec, + next_event_id: usize, flush_task: Option>, } +const AMPLITUDE_EVENTS_URL: &'static str = "https//api2.amplitude.com/batch"; + #[derive(Serialize)] -struct RecordEventParams { - token: &'static str, - device_id: Option, - app_version: Option, - os_version: Option, - events: Vec, +struct AmplitudeEventBatch { + api_key: &'static str, + events: Vec, } #[derive(Serialize)] -struct Event { - #[serde(rename = "type")] - kind: String, +struct AmplitudeEvent { + user_id: Option>, + device_id: Option>, + event_type: String, + event_properties: Option>, + user_properties: Option>, + os_name: &'static str, + os_version: Option>, + app_version: Option>, + event_id: usize, + session_id: u128, time: u128, - properties: Option>, } #[cfg(debug_assertions)] @@ -52,7 +62,6 @@ const MAX_QUEUE_LEN: usize = 1; #[cfg(not(debug_assertions))] const MAX_QUEUE_LEN: usize = 10; -const EVENTS_URI: &'static str = "api/telemetry"; const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(30); impl Telemetry { @@ -61,30 +70,52 @@ impl Telemetry { Arc::new(Self { client, executor: cx.background().clone(), + session_id: SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_millis(), state: Mutex::new(TelemetryState { - os_version: platform.os_version().log_err(), - app_version: platform.app_version().log_err(), + os_version: platform + .os_version() + .log_err() + .map(|v| v.to_string().into()), + os_name: platform.os_name().into(), + app_version: platform + .app_version() + .log_err() + .map(|v| v.to_string().into()), device_id: None, queue: Default::default(), flush_task: Default::default(), + next_event_id: 0, + user_id: None, }), }) } pub fn log_event(self: &Arc, kind: &str, properties: Value) { let mut state = self.state.lock(); - state.queue.push(Event { - kind: kind.to_string(), + let event = AmplitudeEvent { + event_type: kind.to_string(), time: SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap() .as_millis(), - properties: if let Value::Object(properties) = properties { + session_id: self.session_id, + event_properties: if let Value::Object(properties) = properties { Some(properties) } else { None }, - }); + user_properties: None, + user_id: state.user_id.clone(), + device_id: state.device_id.clone(), + os_name: state.os_name, + os_version: state.os_version.clone(), + app_version: state.app_version.clone(), + event_id: post_inc(&mut state.next_event_id), + }; + state.queue.push(event); if state.queue.len() >= MAX_QUEUE_LEN { drop(state); self.flush(); @@ -102,21 +133,15 @@ impl Telemetry { let mut state = self.state.lock(); let events = mem::take(&mut state.queue); let client = self.client.clone(); - let app_version = state.app_version; - let os_version = state.os_version; - let device_id = state.device_id.clone(); state.flush_task.take(); self.executor .spawn(async move { - let body = serde_json::to_vec(&RecordEventParams { - token: ZED_SECRET_CLIENT_TOKEN, + let body = serde_json::to_vec(&AmplitudeEventBatch { + api_key: ZED_SECRET_CLIENT_TOKEN, events, - app_version: app_version.map(|v| v.to_string()), - os_version: os_version.map(|v| v.to_string()), - device_id, }) .log_err()?; - let request = Request::post(format!("{}/{}", *ZED_SERVER_URL, EVENTS_URI)) + let request = Request::post(AMPLITUDE_EVENTS_URL) .header("Content-Type", "application/json") .body(body.into()) .log_err()?; diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 7467dad547..8997bde527 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -69,6 +69,7 @@ pub trait Platform: Send + Sync { fn path_for_auxiliary_executable(&self, name: &str) -> Result; fn app_path(&self) -> Result; fn app_version(&self) -> Result; + fn os_name(&self) -> &'static str; fn os_version(&self) -> Result; } diff --git a/crates/gpui/src/platform/mac/platform.rs b/crates/gpui/src/platform/mac/platform.rs index 02fe73504e..628ddde13c 100644 --- a/crates/gpui/src/platform/mac/platform.rs +++ b/crates/gpui/src/platform/mac/platform.rs @@ -750,6 +750,10 @@ impl platform::Platform for MacPlatform { } } + fn os_name(&self) -> &'static str { + "macOS" + } + fn os_version(&self) -> Result { unsafe { let process_info = NSProcessInfo::processInfo(nil); diff --git a/crates/gpui/src/platform/test.rs b/crates/gpui/src/platform/test.rs index 1bb53d49ab..58ef1ffaf2 100644 --- a/crates/gpui/src/platform/test.rs +++ b/crates/gpui/src/platform/test.rs @@ -197,6 +197,10 @@ impl super::Platform for Platform { }) } + fn os_name(&self) -> &'static str { + "test" + } + fn os_version(&self) -> Result { Ok(AppVersion { major: 1, From f0c50c1e0aa081844e42f52efc5da9ba07ef06ca Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 26 Sep 2022 16:37:09 -0600 Subject: [PATCH 15/22] Extract db module from project to its own crate This will let us use it from the telemetry crate. Co-authored-by: Joseph Lyons --- Cargo.lock | 14 ++++++++++++++ crates/db/Cargo.toml | 22 ++++++++++++++++++++++ crates/{project => db}/src/db.rs | 0 crates/project/Cargo.toml | 3 +++ crates/project/src/project.rs | 1 - 5 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 crates/db/Cargo.toml rename crates/{project => db}/src/db.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index d11d776732..bf145f0cee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1504,6 +1504,19 @@ dependencies = [ "matches", ] +[[package]] +name = "db" +version = "0.1.0" +dependencies = [ + "anyhow", + "async-trait", + "collections", + "gpui", + "parking_lot 0.11.2", + "rocksdb", + "tempdir", +] + [[package]] name = "deflate" version = "0.8.6" @@ -3950,6 +3963,7 @@ dependencies = [ "client", "clock", "collections", + "db", "fsevent", "futures", "fuzzy", diff --git a/crates/db/Cargo.toml b/crates/db/Cargo.toml new file mode 100644 index 0000000000..f4ed283b6e --- /dev/null +++ b/crates/db/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "db" +version = "0.1.0" +edition = "2021" + +[lib] +path = "src/db.rs" +doctest = false + +[features] +test-support = [] + +[dependencies] +collections = { path = "../collections" } +anyhow = "1.0.57" +async-trait = "0.1" +parking_lot = "0.11.1" +rocksdb = "0.18" + +[dev-dependencies] +gpui = { path = "../gpui", features = ["test-support"] } +tempdir = { version = "0.3.7" } diff --git a/crates/project/src/db.rs b/crates/db/src/db.rs similarity index 100% rename from crates/project/src/db.rs rename to crates/db/src/db.rs diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index eebfc08473..a4ea6f2286 100644 --- a/crates/project/Cargo.toml +++ b/crates/project/Cargo.toml @@ -10,6 +10,7 @@ doctest = false [features] test-support = [ "client/test-support", + "db/test-support", "language/test-support", "settings/test-support", "text/test-support", @@ -20,6 +21,7 @@ text = { path = "../text" } client = { path = "../client" } clock = { path = "../clock" } collections = { path = "../collections" } +db = { path = "../db" } fsevent = { path = "../fsevent" } fuzzy = { path = "../fuzzy" } gpui = { path = "../gpui" } @@ -54,6 +56,7 @@ rocksdb = "0.18" [dev-dependencies] client = { path = "../client", features = ["test-support"] } collections = { path = "../collections", features = ["test-support"] } +db = { path = "../db", features = ["test-support"] } gpui = { path = "../gpui", features = ["test-support"] } language = { path = "../language", features = ["test-support"] } lsp = { path = "../lsp", features = ["test-support"] } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index abb55e49b0..73b5e3595d 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1,4 +1,3 @@ -mod db; pub mod fs; mod ignore; mod lsp_command; From 824fdb54e6db1cf4f9cf06718666ba6434785e96 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 26 Sep 2022 18:18:34 -0600 Subject: [PATCH 16/22] Report editor open and save events to Amplitude Co-authored-by: Max Brunsfeld --- Cargo.lock | 5 ++ crates/client/Cargo.toml | 2 + crates/client/src/client.rs | 17 +++-- crates/client/src/telemetry.rs | 117 +++++++++++++++++++++++++-------- crates/editor/src/editor.rs | 21 ++++++ crates/editor/src/items.rs | 1 + crates/zed/build.rs | 4 ++ crates/zed/src/main.rs | 5 +- 8 files changed, 137 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bf145f0cee..cfe18755dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -945,6 +945,7 @@ dependencies = [ "async-recursion", "async-tungstenite", "collections", + "db", "futures", "gpui", "image", @@ -963,6 +964,7 @@ dependencies = [ "tiny_http", "url", "util", + "uuid 1.1.2", ] [[package]] @@ -6346,6 +6348,9 @@ name = "uuid" version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd6469f4314d5f1ffec476e05f17cc9a78bc7a27a6a857842170bdf8d6f98d2f" +dependencies = [ + "getrandom 0.2.7", +] [[package]] name = "valuable" diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index 5fcff565bb..f61fa1c787 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -12,6 +12,7 @@ test-support = ["collections/test-support", "gpui/test-support", "rpc/test-suppo [dependencies] collections = { path = "../collections" } +db = { path = "../db" } gpui = { path = "../gpui" } util = { path = "../util" } rpc = { path = "../rpc" } @@ -31,6 +32,7 @@ smol = "1.2.5" thiserror = "1.0.29" time = { version = "0.3", features = ["serde", "serde-well-known"] } tiny_http = "0.8" +uuid = { version = "1.1.2", features = ["v4"] } url = "2.2" serde = { version = "*", features = ["derive"] } diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 3d85aea3c5..5d6bef5c23 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -12,6 +12,7 @@ use async_tungstenite::tungstenite::{ error::Error as WebsocketError, http::{Request, StatusCode}, }; +use db::Db; use futures::{future::LocalBoxFuture, FutureExt, SinkExt, StreamExt, TryStreamExt}; use gpui::{ actions, @@ -70,7 +71,7 @@ pub fn init(client: Arc, cx: &mut MutableAppContext) { cx.add_global_action({ let client = client.clone(); move |_: &TestTelemetry, _| { - client.log_event( + client.report_event( "test_telemetry", json!({ "test_property": "test_value" @@ -334,6 +335,7 @@ impl Client { match status { Status::Connected { .. } => { + self.telemetry.set_user_id(self.user_id()); state._reconnect_task = None; } Status::ConnectionLost => { @@ -362,6 +364,7 @@ impl Client { })); } Status::SignedOut | Status::UpgradeRequired => { + self.telemetry.set_user_id(self.user_id()); state._reconnect_task.take(); } _ => {} @@ -619,7 +622,7 @@ impl Client { credentials = read_credentials_from_keychain(cx); read_from_keychain = credentials.is_some(); if read_from_keychain { - self.log_event("read_credentials_from_keychain", Default::default()); + self.report_event("read credentials from keychain", Default::default()); } } if credentials.is_none() { @@ -983,7 +986,7 @@ impl Client { .context("failed to decrypt access token")?; platform.activate(true); - telemetry.log_event("authenticate_with_browser", Default::default()); + telemetry.report_event("authenticate with browser", Default::default()); Ok(Credentials { user_id: user_id.parse()?, @@ -1050,8 +1053,12 @@ impl Client { self.peer.respond_with_error(receipt, error) } - pub fn log_event(&self, kind: &str, properties: Value) { - self.telemetry.log_event(kind, properties) + pub fn start_telemetry(&self, db: Arc) { + self.telemetry.start(db); + } + + pub fn report_event(&self, kind: &str, properties: Value) { + self.telemetry.report_event(kind, properties) } } diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 7eea13a923..63da4eae5c 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -1,10 +1,12 @@ -use crate::{http::HttpClient, ZED_SECRET_CLIENT_TOKEN}; +use crate::http::HttpClient; +use db::Db; use gpui::{ executor::Background, serde_json::{self, value::Map, Value}, AppContext, Task, }; use isahc::Request; +use lazy_static::lazy_static; use parking_lot::Mutex; use serde::Serialize; use std::{ @@ -12,7 +14,8 @@ use std::{ sync::Arc, time::{Duration, SystemTime, UNIX_EPOCH}, }; -use util::{post_inc, ResultExt}; +use util::{post_inc, ResultExt, TryFutureExt}; +use uuid::Uuid; pub struct Telemetry { client: Arc, @@ -33,7 +36,13 @@ struct TelemetryState { flush_task: Option>, } -const AMPLITUDE_EVENTS_URL: &'static str = "https//api2.amplitude.com/batch"; +const AMPLITUDE_EVENTS_URL: &'static str = "https://api2.amplitude.com/batch"; + +lazy_static! { + static ref AMPLITUDE_API_KEY: Option = option_env!("AMPLITUDE_API_KEY") + .map(|key| key.to_string()) + .or(std::env::var("AMPLITUDE_API_KEY").ok()); +} #[derive(Serialize)] struct AmplitudeEventBatch { @@ -62,6 +71,10 @@ const MAX_QUEUE_LEN: usize = 1; #[cfg(not(debug_assertions))] const MAX_QUEUE_LEN: usize = 10; +#[cfg(debug_assertions)] +const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(1); + +#[cfg(not(debug_assertions))] const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(30); impl Telemetry { @@ -93,7 +106,52 @@ impl Telemetry { }) } - pub fn log_event(self: &Arc, kind: &str, properties: Value) { + pub fn start(self: &Arc, db: Arc) { + let this = self.clone(); + self.executor + .spawn( + async move { + let device_id = if let Some(device_id) = db + .read(["device_id"])? + .into_iter() + .flatten() + .next() + .and_then(|bytes| String::from_utf8(bytes).ok()) + { + device_id + } else { + let device_id = Uuid::new_v4().to_string(); + db.write([("device_id", device_id.as_bytes())])?; + device_id + }; + + let device_id = Some(Arc::from(device_id)); + let mut state = this.state.lock(); + state.device_id = device_id.clone(); + for event in &mut state.queue { + event.device_id = device_id.clone(); + } + if !state.queue.is_empty() { + drop(state); + this.flush(); + } + + anyhow::Ok(()) + } + .log_err(), + ) + .detach(); + } + + pub fn set_user_id(&self, user_id: Option) { + self.state.lock().user_id = user_id.map(|id| id.to_string().into()); + } + + pub fn report_event(self: &Arc, kind: &str, properties: Value) { + if AMPLITUDE_API_KEY.is_none() { + return; + } + let mut state = self.state.lock(); let event = AmplitudeEvent { event_type: kind.to_string(), @@ -116,38 +174,39 @@ impl Telemetry { event_id: post_inc(&mut state.next_event_id), }; state.queue.push(event); - if state.queue.len() >= MAX_QUEUE_LEN { - drop(state); - self.flush(); - } else { - let this = self.clone(); - let executor = self.executor.clone(); - state.flush_task = Some(self.executor.spawn(async move { - executor.timer(DEBOUNCE_INTERVAL).await; - this.flush(); - })); + if state.device_id.is_some() { + if state.queue.len() >= MAX_QUEUE_LEN { + drop(state); + self.flush(); + } else { + let this = self.clone(); + let executor = self.executor.clone(); + state.flush_task = Some(self.executor.spawn(async move { + executor.timer(DEBOUNCE_INTERVAL).await; + this.flush(); + })); + } } } fn flush(&self) { let mut state = self.state.lock(); let events = mem::take(&mut state.queue); - let client = self.client.clone(); state.flush_task.take(); - self.executor - .spawn(async move { - let body = serde_json::to_vec(&AmplitudeEventBatch { - api_key: ZED_SECRET_CLIENT_TOKEN, - events, + + if let Some(api_key) = AMPLITUDE_API_KEY.as_ref() { + let client = self.client.clone(); + self.executor + .spawn(async move { + let batch = AmplitudeEventBatch { api_key, events }; + let body = serde_json::to_vec(&batch).log_err()?; + let request = Request::post(AMPLITUDE_EVENTS_URL) + .body(body.into()) + .log_err()?; + client.send(request).await.log_err(); + Some(()) }) - .log_err()?; - let request = Request::post(AMPLITUDE_EVENTS_URL) - .header("Content-Type", "application/json") - .body(body.into()) - .log_err()?; - client.send(request).await.log_err(); - Some(()) - }) - .detach(); + .detach(); + } } } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index c60abc187a..07a9fc011f 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -29,6 +29,7 @@ use gpui::{ geometry::vector::{vec2f, Vector2F}, impl_actions, impl_internal_actions, platform::CursorStyle, + serde_json::json, text_layout, AnyViewHandle, AppContext, AsyncAppContext, ClipboardItem, Element, ElementBox, Entity, ModelHandle, MouseButton, MutableAppContext, RenderContext, Subscription, Task, View, ViewContext, ViewHandle, WeakViewHandle, @@ -1053,6 +1054,7 @@ impl Editor { let editor_created_event = EditorCreated(cx.handle()); cx.emit_global(editor_created_event); + this.report_event("open editor", cx); this } @@ -5928,6 +5930,25 @@ impl Editor { }) .collect() } + + fn report_event(&self, name: &str, cx: &AppContext) { + if let Some((project, file)) = self.project.as_ref().zip( + self.buffer + .read(cx) + .as_singleton() + .and_then(|b| b.read(cx).file()), + ) { + project.read(cx).client().report_event( + name, + json!({ + "file_extension": file + .path() + .extension() + .and_then(|e| e.to_str()) + }), + ); + } + } } impl EditorSnapshot { diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index fb6f12a16f..6c004f2007 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -410,6 +410,7 @@ impl Item for Editor { let buffers = buffer.read(cx).all_buffers(); let mut timeout = cx.background().timer(FORMAT_TIMEOUT).fuse(); let format = project.update(cx, |project, cx| project.format(buffers, true, cx)); + self.report_event("save editor", cx); cx.spawn(|_, mut cx| async move { let transaction = futures::select_biased! { _ = timeout => { diff --git a/crates/zed/build.rs b/crates/zed/build.rs index e39946876e..0ffa2397b0 100644 --- a/crates/zed/build.rs +++ b/crates/zed/build.rs @@ -3,6 +3,10 @@ use std::process::Command; fn main() { println!("cargo:rustc-env=MACOSX_DEPLOYMENT_TARGET=10.14"); + if let Ok(api_key) = std::env::var("AMPLITUDE_API_KEY") { + println!("cargo:rustc-env=AMPLITUDE_API_KEY={api_key}"); + } + let output = Command::new("npm") .current_dir("../../styles") .args(["install", "--no-save"]) diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index bb913ab610..2dd90eb762 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -121,7 +121,6 @@ fn main() { vim::init(cx); terminal::init(cx); - let db = cx.background().block(db); cx.spawn(|cx| watch_themes(fs.clone(), themes.clone(), cx)) .detach(); @@ -139,6 +138,10 @@ fn main() { }) .detach(); + let db = cx.background().block(db); + client.start_telemetry(db.clone()); + client.report_event("start app", Default::default()); + let project_store = cx.add_model(|_| ProjectStore::new(db.clone())); let app_state = Arc::new(AppState { languages, From f2db3abdb28b6700a4cbad51345080333c6c0701 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Sep 2022 12:42:27 -0700 Subject: [PATCH 17/22] Always allow overriding amplitude API key via a runtime env var --- crates/client/src/telemetry.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 63da4eae5c..f048dfdd49 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -39,9 +39,9 @@ struct TelemetryState { const AMPLITUDE_EVENTS_URL: &'static str = "https://api2.amplitude.com/batch"; lazy_static! { - static ref AMPLITUDE_API_KEY: Option = option_env!("AMPLITUDE_API_KEY") - .map(|key| key.to_string()) - .or(std::env::var("AMPLITUDE_API_KEY").ok()); + static ref AMPLITUDE_API_KEY: Option = std::env::var("ZED_AMPLITUDE_API_KEY") + .ok() + .or_else(|| option_env!("ZED_AMPLITUDE_API_KEY").map(|key| key.to_string())); } #[derive(Serialize)] From 3bd68128d7ab67c1b785f4a631caaeccfb056277 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Sep 2022 14:20:13 -0700 Subject: [PATCH 18/22] Add command to view the telemetry log Co-authored-by: Joseph Lyons --- Cargo.lock | 1 + crates/client/Cargo.toml | 1 + crates/client/src/client.rs | 10 ++++- crates/client/src/telemetry.rs | 73 +++++++++++++++++++++++++++------- crates/zed/src/menus.rs | 5 +++ crates/zed/src/zed.rs | 54 +++++++++++++++++++++++++ 6 files changed, 127 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cfe18755dd..17ace3f47b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -959,6 +959,7 @@ dependencies = [ "serde", "smol", "sum_tree", + "tempfile", "thiserror", "time 0.3.11", "tiny_http", diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index f61fa1c787..c9c783c659 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -35,6 +35,7 @@ tiny_http = "0.8" uuid = { version = "1.1.2", features = ["v4"] } url = "2.2" serde = { version = "*", features = ["derive"] } +tempfile = "3" [dev-dependencies] collections = { path = "../collections", features = ["test-support"] } diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 5d6bef5c23..0670add1af 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -33,6 +33,7 @@ use std::{ convert::TryFrom, fmt::Write as _, future::Future, + path::PathBuf, sync::{Arc, Weak}, time::{Duration, Instant}, }; @@ -332,10 +333,11 @@ impl Client { log::info!("set status on client {}: {:?}", self.id, status); let mut state = self.state.write(); *state.status.0.borrow_mut() = status; + let user_id = state.credentials.as_ref().map(|c| c.user_id); match status { Status::Connected { .. } => { - self.telemetry.set_user_id(self.user_id()); + self.telemetry.set_user_id(user_id); state._reconnect_task = None; } Status::ConnectionLost => { @@ -364,7 +366,7 @@ impl Client { })); } Status::SignedOut | Status::UpgradeRequired => { - self.telemetry.set_user_id(self.user_id()); + self.telemetry.set_user_id(user_id); state._reconnect_task.take(); } _ => {} @@ -1060,6 +1062,10 @@ impl Client { pub fn report_event(&self, kind: &str, properties: Value) { self.telemetry.report_event(kind, properties) } + + pub fn telemetry_log_file_path(&self) -> Option { + self.telemetry.log_file_path() + } } impl AnyWeakEntityHandle { diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index f048dfdd49..77aa308f30 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -10,15 +10,18 @@ use lazy_static::lazy_static; use parking_lot::Mutex; use serde::Serialize; use std::{ + io::Write, mem, + path::PathBuf, sync::Arc, time::{Duration, SystemTime, UNIX_EPOCH}, }; +use tempfile::NamedTempFile; use util::{post_inc, ResultExt, TryFutureExt}; use uuid::Uuid; pub struct Telemetry { - client: Arc, + http_client: Arc, executor: Arc, session_id: u128, state: Mutex, @@ -34,6 +37,7 @@ struct TelemetryState { queue: Vec, next_event_id: usize, flush_task: Option>, + log_file: Option, } const AMPLITUDE_EVENTS_URL: &'static str = "https://api2.amplitude.com/batch"; @@ -52,10 +56,13 @@ struct AmplitudeEventBatch { #[derive(Serialize)] struct AmplitudeEvent { + #[serde(skip_serializing_if = "Option::is_none")] user_id: Option>, device_id: Option>, event_type: String, + #[serde(skip_serializing_if = "Option::is_none")] event_properties: Option>, + #[serde(skip_serializing_if = "Option::is_none")] user_properties: Option>, os_name: &'static str, os_version: Option>, @@ -80,8 +87,8 @@ const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(30); impl Telemetry { pub fn new(client: Arc, cx: &AppContext) -> Arc { let platform = cx.platform(); - Arc::new(Self { - client, + let this = Arc::new(Self { + http_client: client, executor: cx.background().clone(), session_id: SystemTime::now() .duration_since(UNIX_EPOCH) @@ -101,9 +108,29 @@ impl Telemetry { queue: Default::default(), flush_task: Default::default(), next_event_id: 0, + log_file: None, user_id: None, }), - }) + }); + + if AMPLITUDE_API_KEY.is_some() { + this.executor + .spawn({ + let this = this.clone(); + async move { + if let Some(tempfile) = NamedTempFile::new().log_err() { + this.state.lock().log_file = Some(tempfile); + } + } + }) + .detach(); + } + + this + } + + pub fn log_file_path(&self) -> Option { + Some(self.state.lock().log_file.as_ref()?.path().to_path_buf()) } pub fn start(self: &Arc, db: Arc) { @@ -189,23 +216,39 @@ impl Telemetry { } } - fn flush(&self) { + fn flush(self: &Arc) { let mut state = self.state.lock(); let events = mem::take(&mut state.queue); state.flush_task.take(); + drop(state); if let Some(api_key) = AMPLITUDE_API_KEY.as_ref() { - let client = self.client.clone(); + let this = self.clone(); self.executor - .spawn(async move { - let batch = AmplitudeEventBatch { api_key, events }; - let body = serde_json::to_vec(&batch).log_err()?; - let request = Request::post(AMPLITUDE_EVENTS_URL) - .body(body.into()) - .log_err()?; - client.send(request).await.log_err(); - Some(()) - }) + .spawn( + async move { + let mut json_bytes = Vec::new(); + + if let Some(file) = &mut this.state.lock().log_file { + let file = file.as_file_mut(); + for event in &events { + json_bytes.clear(); + serde_json::to_writer(&mut json_bytes, event)?; + file.write_all(&json_bytes)?; + file.write(b"\n")?; + } + } + + let batch = AmplitudeEventBatch { api_key, events }; + json_bytes.clear(); + serde_json::to_writer(&mut json_bytes, &batch)?; + let request = + Request::post(AMPLITUDE_EVENTS_URL).body(json_bytes.into())?; + this.http_client.send(request).await?; + Ok(()) + } + .log_err(), + ) .detach(); } } diff --git a/crates/zed/src/menus.rs b/crates/zed/src/menus.rs index 3a34166ba6..f21845a589 100644 --- a/crates/zed/src/menus.rs +++ b/crates/zed/src/menus.rs @@ -332,6 +332,11 @@ pub fn menus() -> Vec> { action: Box::new(command_palette::Toggle), }, MenuItem::Separator, + MenuItem::Action { + name: "View Telemetry Log", + action: Box::new(crate::OpenTelemetryLog), + }, + MenuItem::Separator, MenuItem::Action { name: "Documentation", action: Box::new(crate::OpenBrowser { diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index cd906500ee..407f101421 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -56,6 +56,7 @@ actions!( DebugElements, OpenSettings, OpenLog, + OpenTelemetryLog, OpenKeymap, OpenDefaultSettings, OpenDefaultKeymap, @@ -146,6 +147,12 @@ pub fn init(app_state: &Arc, cx: &mut gpui::MutableAppContext) { open_log_file(workspace, app_state.clone(), cx); } }); + cx.add_action({ + let app_state = app_state.clone(); + move |workspace: &mut Workspace, _: &OpenTelemetryLog, cx: &mut ViewContext| { + open_telemetry_log_file(workspace, app_state.clone(), cx); + } + }); cx.add_action({ let app_state = app_state.clone(); move |_: &mut Workspace, _: &OpenKeymap, cx: &mut ViewContext| { @@ -504,6 +511,53 @@ fn open_log_file( }); } +fn open_telemetry_log_file( + workspace: &mut Workspace, + app_state: Arc, + cx: &mut ViewContext, +) { + workspace.with_local_workspace(cx, app_state.clone(), |_, cx| { + cx.spawn_weak(|workspace, mut cx| async move { + let workspace = workspace.upgrade(&cx)?; + let path = app_state.client.telemetry_log_file_path()?; + let log = app_state.fs.load(&path).await.log_err()?; + workspace.update(&mut cx, |workspace, cx| { + let project = workspace.project().clone(); + let buffer = project + .update(cx, |project, cx| project.create_buffer("", None, cx)) + .expect("creating buffers on a local workspace always succeeds"); + buffer.update(cx, |buffer, cx| { + buffer.set_language(app_state.languages.get_language("JSON"), cx); + buffer.edit( + [( + 0..0, + concat!( + "// Zed collects anonymous usage data to help us understand how people are using the app.\n", + "// After the beta release, we'll provide the ability to opt out of this telemetry.\n", + "\n" + ), + )], + None, + cx, + ); + buffer.edit([(buffer.len()..buffer.len(), log)], None, cx); + }); + + let buffer = cx.add_model(|cx| { + MultiBuffer::singleton(buffer, cx).with_title("Telemetry Log".into()) + }); + workspace.add_item( + Box::new(cx.add_view(|cx| Editor::for_multibuffer(buffer, Some(project), cx))), + cx, + ); + }); + + Some(()) + }) + .detach(); + }); +} + fn open_bundled_config_file( workspace: &mut Workspace, app_state: Arc, From c1c5eaeaf998cc0667d1164d909c02eaf93278f7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Sep 2022 14:25:28 -0700 Subject: [PATCH 19/22] Use the amplitude API key secret on CI Co-authored-by: Joseph Lyons --- .github/workflows/ci.yml | 1 + crates/zed/build.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cef9497074..866d0acc0e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,6 +56,7 @@ jobs: MACOS_CERTIFICATE_PASSWORD: ${{ secrets.MACOS_CERTIFICATE_PASSWORD }} APPLE_NOTARIZATION_USERNAME: ${{ secrets.APPLE_NOTARIZATION_USERNAME }} APPLE_NOTARIZATION_PASSWORD: ${{ secrets.APPLE_NOTARIZATION_PASSWORD }} + ZED_AMPLITUDE_API_KEY: ${{ secrets.ZED_AMPLITUDE_API_KEY }} steps: - name: Install Rust run: | diff --git a/crates/zed/build.rs b/crates/zed/build.rs index 0ffa2397b0..d3167851a0 100644 --- a/crates/zed/build.rs +++ b/crates/zed/build.rs @@ -3,8 +3,8 @@ use std::process::Command; fn main() { println!("cargo:rustc-env=MACOSX_DEPLOYMENT_TARGET=10.14"); - if let Ok(api_key) = std::env::var("AMPLITUDE_API_KEY") { - println!("cargo:rustc-env=AMPLITUDE_API_KEY={api_key}"); + if let Ok(api_key) = std::env::var("ZED_AMPLITUDE_API_KEY") { + println!("cargo:rustc-env=ZED_AMPLITUDE_API_KEY={api_key}"); } let output = Command::new("npm") From ac0bcf3809b7ad336ee558fa247885680ad0366e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Sep 2022 15:09:16 -0700 Subject: [PATCH 20/22] Limit the size of the buffer in the OpenTelemetryLog command Co-authored-by: Joseph Lyons --- crates/zed/src/zed.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 407f101421..76bc62e4cb 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -521,6 +521,14 @@ fn open_telemetry_log_file( let workspace = workspace.upgrade(&cx)?; let path = app_state.client.telemetry_log_file_path()?; let log = app_state.fs.load(&path).await.log_err()?; + + const MAX_TELEMETRY_LOG_LEN: usize = 5 * 1024 * 1024; + let mut start_offset = log.len().saturating_sub(MAX_TELEMETRY_LOG_LEN); + if let Some(newline_offset) = log[start_offset..].find('\n') { + start_offset += newline_offset + 1; + } + let log_suffix = &log[start_offset..]; + workspace.update(&mut cx, |workspace, cx| { let project = workspace.project().clone(); let buffer = project @@ -534,13 +542,14 @@ fn open_telemetry_log_file( concat!( "// Zed collects anonymous usage data to help us understand how people are using the app.\n", "// After the beta release, we'll provide the ability to opt out of this telemetry.\n", + "// Here is the data that has been reported for the current session:\n", "\n" ), )], None, cx, ); - buffer.edit([(buffer.len()..buffer.len(), log)], None, cx); + buffer.edit([(buffer.len()..buffer.len(), log_suffix)], None, cx); }); let buffer = cx.add_model(|cx| { From 1db75ca2cfd8bffc9594a023c9d0c39dba816348 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Sep 2022 15:49:13 -0700 Subject: [PATCH 21/22] Make device_id optional on signups table This way, signup won't fail if for some reason, the user's client-side JS doesn't provide an amplitude device id. Co-authored-by: Joseph Lyons --- .../20220913211150_create_signups.up.sql | 2 +- crates/collab/src/api.rs | 13 ++++-- crates/collab/src/db.rs | 46 ++++++++++++++----- crates/collab/src/db_tests.rs | 45 ++++++++++++------ 4 files changed, 75 insertions(+), 31 deletions(-) diff --git a/crates/collab/migrations/20220913211150_create_signups.up.sql b/crates/collab/migrations/20220913211150_create_signups.up.sql index 35e334ea5f..19559b747c 100644 --- a/crates/collab/migrations/20220913211150_create_signups.up.sql +++ b/crates/collab/migrations/20220913211150_create_signups.up.sql @@ -4,7 +4,7 @@ CREATE TABLE IF NOT EXISTS "signups" ( "email_confirmation_code" VARCHAR(64) NOT NULL, "email_confirmation_sent" BOOLEAN NOT NULL, "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, - "device_id" VARCHAR NOT NULL, + "device_id" VARCHAR, "user_id" INTEGER REFERENCES users (id) ON DELETE CASCADE, "inviting_user_id" INTEGER REFERENCES users (id) ON DELETE SET NULL, diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index a82363a56b..0a9d8106ce 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -157,9 +157,9 @@ async fn create_user( user, ) .await?; - user_id = result.0; - signup_device_id = Some(result.2); - if let Some(inviter_id) = result.1 { + user_id = result.user_id; + signup_device_id = result.signup_device_id; + if let Some(inviter_id) = result.inviting_user_id { rpc_server .invite_code_redeemed(inviter_id, user_id) .await @@ -425,6 +425,7 @@ async fn get_waitlist_summary( pub struct CreateInviteFromCodeParams { invite_code: String, email_address: String, + device_id: Option, } async fn create_invite_from_code( @@ -433,7 +434,11 @@ async fn create_invite_from_code( ) -> Result> { Ok(Json( app.db - .create_invite_from_code(¶ms.invite_code, ¶ms.email_address) + .create_invite_from_code( + ¶ms.invite_code, + ¶ms.email_address, + params.device_id.as_deref(), + ) .await?, )) } diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 1518ec179f..8b01cdf971 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -35,7 +35,12 @@ pub trait Db: Send + Sync { async fn set_invite_count_for_user(&self, id: UserId, count: u32) -> Result<()>; async fn get_invite_code_for_user(&self, id: UserId) -> Result>; async fn get_user_for_invite_code(&self, code: &str) -> Result; - async fn create_invite_from_code(&self, code: &str, email_address: &str) -> Result; + async fn create_invite_from_code( + &self, + code: &str, + email_address: &str, + device_id: Option<&str>, + ) -> Result; async fn create_signup(&self, signup: Signup) -> Result<()>; async fn get_waitlist_summary(&self) -> Result; @@ -45,7 +50,7 @@ pub trait Db: Send + Sync { &self, invite: &Invite, user: NewUserParams, - ) -> Result<(UserId, Option, String)>; + ) -> Result; /// Registers a new project for the given user. async fn register_project(&self, host_user_id: UserId) -> Result; @@ -458,14 +463,14 @@ impl Db for PostgresDb { &self, invite: &Invite, user: NewUserParams, - ) -> Result<(UserId, Option, String)> { + ) -> Result { let mut tx = self.pool.begin().await?; - let (signup_id, existing_user_id, inviting_user_id, device_id): ( + let (signup_id, existing_user_id, inviting_user_id, signup_device_id): ( i32, Option, Option, - String, + Option, ) = sqlx::query_as( " SELECT id, user_id, inviting_user_id, device_id @@ -552,7 +557,11 @@ impl Db for PostgresDb { } tx.commit().await?; - Ok((user_id, inviting_user_id, device_id)) + Ok(NewUserResult { + user_id, + inviting_user_id, + signup_device_id, + }) } // invite codes @@ -625,7 +634,12 @@ impl Db for PostgresDb { }) } - async fn create_invite_from_code(&self, code: &str, email_address: &str) -> Result { + async fn create_invite_from_code( + &self, + code: &str, + email_address: &str, + device_id: Option<&str>, + ) -> Result { let mut tx = self.pool.begin().await?; let existing_user: Option = sqlx::query_scalar( @@ -679,10 +693,11 @@ impl Db for PostgresDb { platform_linux, platform_mac, platform_windows, - platform_unknown + platform_unknown, + device_id ) VALUES - ($1, $2, 'f', $3, 'f', 'f', 'f', 't') + ($1, $2, 'f', $3, 'f', 'f', 'f', 't', $4) ON CONFLICT (email_address) DO UPDATE SET inviting_user_id = excluded.inviting_user_id @@ -692,6 +707,7 @@ impl Db for PostgresDb { .bind(&email_address) .bind(&random_email_confirmation_code()) .bind(&inviter_id) + .bind(&device_id) .fetch_one(&mut tx) .await?; @@ -1675,7 +1691,7 @@ pub struct Signup { pub platform_linux: bool, pub editor_features: Vec, pub programming_languages: Vec, - pub device_id: String, + pub device_id: Option, } #[derive(Clone, Debug, PartialEq, Deserialize, Serialize, FromRow)] @@ -1703,6 +1719,13 @@ pub struct NewUserParams { pub invite_count: i32, } +#[derive(Debug)] +pub struct NewUserResult { + pub user_id: UserId, + pub inviting_user_id: Option, + pub signup_device_id: Option, +} + fn random_invite_code() -> String { nanoid::nanoid!(16) } @@ -1905,7 +1928,7 @@ mod test { &self, _invite: &Invite, _user: NewUserParams, - ) -> Result<(UserId, Option, String)> { + ) -> Result { unimplemented!() } @@ -1928,6 +1951,7 @@ mod test { &self, _code: &str, _email_address: &str, + _device_id: Option<&str>, ) -> Result { unimplemented!() } diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index 44697a59bd..1e48b4b754 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -954,10 +954,14 @@ async fn test_invite_codes() { // User 2 redeems the invite code and becomes a contact of user 1. let user2_invite = db - .create_invite_from_code(&invite_code, "u2@example.com") + .create_invite_from_code(&invite_code, "u2@example.com", Some("user-2-device-id")) .await .unwrap(); - let (user2, inviter, _) = db + let NewUserResult { + user_id: user2, + inviting_user_id, + signup_device_id, + } = db .create_user_from_invite( &user2_invite, NewUserParams { @@ -970,7 +974,8 @@ async fn test_invite_codes() { .unwrap(); let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); assert_eq!(invite_count, 1); - assert_eq!(inviter, Some(user1)); + assert_eq!(inviting_user_id, Some(user1)); + assert_eq!(signup_device_id.unwrap(), "user-2-device-id"); assert_eq!( db.get_contacts(user1).await.unwrap(), [ @@ -1004,10 +1009,14 @@ async fn test_invite_codes() { // User 3 redeems the invite code and becomes a contact of user 1. let user3_invite = db - .create_invite_from_code(&invite_code, "u3@example.com") + .create_invite_from_code(&invite_code, "u3@example.com", None) .await .unwrap(); - let (user3, inviter, _) = db + let NewUserResult { + user_id: user3, + inviting_user_id, + signup_device_id, + } = db .create_user_from_invite( &user3_invite, NewUserParams { @@ -1020,7 +1029,8 @@ async fn test_invite_codes() { .unwrap(); let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); assert_eq!(invite_count, 0); - assert_eq!(inviter, Some(user1)); + assert_eq!(inviting_user_id, Some(user1)); + assert!(signup_device_id.is_none()); assert_eq!( db.get_contacts(user1).await.unwrap(), [ @@ -1057,7 +1067,7 @@ async fn test_invite_codes() { ); // Trying to reedem the code for the third time results in an error. - db.create_invite_from_code(&invite_code, "u4@example.com") + db.create_invite_from_code(&invite_code, "u4@example.com", Some("user-4-device-id")) .await .unwrap_err(); @@ -1069,10 +1079,10 @@ async fn test_invite_codes() { // User 4 can now redeem the invite code and becomes a contact of user 1. let user4_invite = db - .create_invite_from_code(&invite_code, "u4@example.com") + .create_invite_from_code(&invite_code, "u4@example.com", Some("user-4-device-id")) .await .unwrap(); - let (user4, _, _) = db + let user4 = db .create_user_from_invite( &user4_invite, NewUserParams { @@ -1082,7 +1092,8 @@ async fn test_invite_codes() { }, ) .await - .unwrap(); + .unwrap() + .user_id; let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); assert_eq!(invite_count, 1); @@ -1126,7 +1137,7 @@ async fn test_invite_codes() { ); // An existing user cannot redeem invite codes. - db.create_invite_from_code(&invite_code, "u2@example.com") + db.create_invite_from_code(&invite_code, "u2@example.com", Some("user-2-device-id")) .await .unwrap_err(); let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); @@ -1147,7 +1158,7 @@ async fn test_signups() { platform_windows: i % 4 == 0, editor_features: vec!["speed".into()], programming_languages: vec!["rust".into(), "c".into()], - device_id: format!("device_id_{i}"), + device_id: Some(format!("device_id_{i}")), }) .await .unwrap(); @@ -1217,7 +1228,11 @@ async fn test_signups() { // user completes the signup process by providing their // github account. - let (user_id, inviter_id, signup_device_id) = db + let NewUserResult { + user_id, + inviting_user_id, + signup_device_id, + } = db .create_user_from_invite( &Invite { email_address: signups_batch1[0].email_address.clone(), @@ -1232,11 +1247,11 @@ async fn test_signups() { .await .unwrap(); let user = db.get_user_by_id(user_id).await.unwrap().unwrap(); - assert!(inviter_id.is_none()); + assert!(inviting_user_id.is_none()); assert_eq!(user.github_login, "person-0"); assert_eq!(user.email_address.as_deref(), Some("person-0@example.com")); assert_eq!(user.invite_count, 5); - assert_eq!(signup_device_id, "device_id_0"); + assert_eq!(signup_device_id.unwrap(), "device_id_0"); // cannot redeem the same signup again. db.create_user_from_invite( From f2ebb094a2bd1541dadd27a1e2ce4b2be21a76ea Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Sep 2022 16:58:03 -0700 Subject: [PATCH 22/22] Remove unnecessary index drop in down migration --- crates/collab/migrations/20220913211150_create_signups.down.sql | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/collab/migrations/20220913211150_create_signups.down.sql b/crates/collab/migrations/20220913211150_create_signups.down.sql index f67c10dd01..5504bbb8dc 100644 --- a/crates/collab/migrations/20220913211150_create_signups.down.sql +++ b/crates/collab/migrations/20220913211150_create_signups.down.sql @@ -4,4 +4,3 @@ ALTER TABLE users DROP COLUMN github_user_id; DROP INDEX index_users_on_email_address; -DROP INDEX index_users_on_github_user_id;