Correct error messages if user registers with taken user/email (#3093)

* Correct error messages if user registers with taken user/email (fixes #2955)

* exists

crates/api_crud/src/user/create.rs

@@ -83,6 +83,12 @@ impl PerformCrud for Register {
       &context.settings().get_protocol_and_hostname(),
     )?;
 
+    if let Some(email) = &data.email {
+      if LocalUser::is_email_taken(context.pool(), email).await? {
+        return Err(LemmyError::from_message("email_already_exists"));
+      }
+    }
+
     // We have to create both a person, and local_user
 
     // Register the new person
@@ -116,23 +122,7 @@ impl PerformCrud for Register {
       .accepted_application(accepted_application)
       .build();
 
-    let inserted_local_user = match LocalUser::create(context.pool(), &local_user_form).await {
-      Ok(lu) => lu,
-      Err(e) => {
-        let err_type = if e.to_string()
-          == "duplicate key value violates unique constraint \"local_user_email_key\""
-        {
-          "email_already_exists"
-        } else {
-          "user_already_exists"
-        };
-
-        // If the local user creation errored, then delete that person
-        Person::delete(context.pool(), inserted_person.id).await?;
-
-        return Err(LemmyError::from_error_message(e, err_type));
-      }
-    };
+    let inserted_local_user = LocalUser::create(context.pool(), &local_user_form).await?;
 
     if local_site.site_setup && require_registration_application {
       // Create the registration application

crates/apub/src/objects/person.rs

@@ -171,7 +171,7 @@ impl Object for ApubPerson {
       matrix_user_id: person.matrix_user_id,
       instance_id,
     };
-    let person = DbPerson::create(context.pool(), &person_form).await?;
+    let person = DbPerson::upsert(context.pool(), &person_form).await?;
 
     Ok(person.into())
   }

crates/db_schema/src/impls/local_user.rs

@@ -2,6 +2,7 @@ use crate::{
   newtypes::LocalUserId,
   schema::local_user::dsl::{
     accepted_application,
+    email,
     email_verified,
     local_user,
     password_encrypted,
@@ -53,6 +54,14 @@ impl LocalUser {
       .get_results::<Self>(conn)
       .await
   }
+
+  pub async fn is_email_taken(pool: &DbPool, email_: &str) -> Result<bool, Error> {
+    use diesel::dsl::{exists, select};
+    let conn = &mut get_conn(pool).await?;
+    select(exists(local_user.filter(email.eq(email_))))
+      .get_result(conn)
+      .await
+  }
 }
 
 #[async_trait]
@@ -80,8 +89,7 @@ impl Crud for LocalUser {
     let local_user_ = insert_into(local_user)
       .values(form_with_encrypted_password)
       .get_result::<Self>(conn)
-      .await
-      .expect("couldnt create local user");
+      .await?;
 
     let site_languages = SiteLanguage::read_local_raw(pool).await;
     if let Ok(langs) = site_languages {

crates/db_schema/src/impls/person.rs

@@ -37,9 +37,6 @@ impl Crud for Person {
     let conn = &mut get_conn(pool).await?;
     insert_into(person::table)
       .values(form)
-      .on_conflict(person::actor_id)
-      .do_update()
-      .set(form)
       .get_result::<Self>(conn)
       .await
   }
@@ -57,6 +54,19 @@ impl Crud for Person {
 }
 
 impl Person {
+  /// Update or insert the person.
+  ///
+  /// This is necessary for federation, because Activitypub doesnt distinguish between these actions.
+  pub async fn upsert(pool: &DbPool, form: &PersonInsertForm) -> Result<Self, Error> {
+    let conn = &mut get_conn(pool).await?;
+    insert_into(person::table)
+      .values(form)
+      .on_conflict(person::actor_id)
+      .do_update()
+      .set(form)
+      .get_result::<Self>(conn)
+      .await
+  }
   pub async fn delete_account(pool: &DbPool, person_id: PersonId) -> Result<Person, Error> {
     let conn = &mut get_conn(pool).await?;