From 6b83deb339d518c35f8cf37d9c2c440a8f1035d8 Mon Sep 17 00:00:00 2001 From: a-maurice Date: Tue, 1 Oct 2024 16:02:33 -0700 Subject: [PATCH 1/9] Add tests around updating user profile --- auth/integration_test/src/integration_test.cc | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/auth/integration_test/src/integration_test.cc b/auth/integration_test/src/integration_test.cc index 597919f6bf..e026803adb 100644 --- a/auth/integration_test/src/integration_test.cc +++ b/auth/integration_test/src/integration_test.cc @@ -671,6 +671,78 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfile) { DeleteUser(); } +TEST_F(FirebaseAuthTest, TestUpdateUserProfileNull) { + std::string email = GenerateEmailAddress(); + firebase::Future create_user = + auth_->CreateUserWithEmailAndPassword(email.c_str(), kTestPassword); + WaitForCompletion(create_user, "CreateUserWithEmailAndPassword"); + EXPECT_TRUE(auth_->current_user().is_valid()); + // Set some user profile properties. + firebase::auth::User user = create_user.result()->user; + const char kDisplayName[] = "Hello World"; + const char kPhotoUrl[] = "http://example.com/image.jpg"; + firebase::auth::User::UserProfile user_profile; + user_profile.display_name = kDisplayName; + user_profile.photo_url = kPhotoUrl; + firebase::Future update_profile = user.UpdateUserProfile(user_profile); + WaitForCompletion(update_profile, "UpdateUserProfile"); + user = auth_->current_user(); + EXPECT_EQ(user.display_name(), kDisplayName); + EXPECT_EQ(user.photo_url(), kPhotoUrl); + firebase::auth::User::UserProfile user_profile_null; + user_profile_null.display_name = kDisplayName; + user_profile_null.photo_url = null; + firebase::Future update_profile_null = user.UpdateUserProfile(user_profile_null); + WaitForCompletion(user_profile_null, "UpdateUserProfileNull"); + user = auth_->current_user(); + EXPECT_EQ(user.display_name(), kDisplayName); + EXPECT_EQ(user.photo_url(), null); + SignOut(); + WaitForCompletion( + auth_->SignInWithEmailAndPassword(email.c_str(), kTestPassword), + "SignInWithEmailAndPassword"); + user = auth_->current_user(); + EXPECT_EQ(user.display_name(), kDisplayName); + EXPECT_EQ(user.photo_url(), null); + DeleteUser(); +} + +TEST_F(FirebaseAuthTest, TestUpdateUserProfileEmpty) { + std::string email = GenerateEmailAddress(); + firebase::Future create_user = + auth_->CreateUserWithEmailAndPassword(email.c_str(), kTestPassword); + WaitForCompletion(create_user, "CreateUserWithEmailAndPassword"); + EXPECT_TRUE(auth_->current_user().is_valid()); + // Set some user profile properties. + firebase::auth::User user = create_user.result()->user; + const char kDisplayName[] = "Hello World"; + const char kPhotoUrl[] = "http://example.com/image.jpg"; + firebase::auth::User::UserProfile user_profile; + user_profile.display_name = kDisplayName; + user_profile.photo_url = kPhotoUrl; + firebase::Future update_profile = user.UpdateUserProfile(user_profile); + WaitForCompletion(update_profile, "UpdateUserProfile"); + user = auth_->current_user(); + EXPECT_EQ(user.display_name(), kDisplayName); + EXPECT_EQ(user.photo_url(), kPhotoUrl); + firebase::auth::User::UserProfile user_profile_null; + user_profile_null.display_name = kDisplayName; + user_profile_null.photo_url = ""; + firebase::Future update_profile_null = user.UpdateUserProfile(user_profile_null); + WaitForCompletion(user_profile_null, "UpdateUserProfileEmpty"); + user = auth_->current_user(); + EXPECT_EQ(user.display_name(), kDisplayName); + EXPECT_EQ(user.photo_url(), ""); + SignOut(); + WaitForCompletion( + auth_->SignInWithEmailAndPassword(email.c_str(), kTestPassword), + "SignInWithEmailAndPassword"); + user = auth_->current_user(); + EXPECT_EQ(user.display_name(), kDisplayName); + EXPECT_EQ(user.photo_url(), ""); + DeleteUser(); +} + TEST_F(FirebaseAuthTest, TestUpdateEmailAndPassword) { std::string email = GenerateEmailAddress(); WaitForCompletion( From 0ff02b40ce4e330a0484a6b726f874a573f28771 Mon Sep 17 00:00:00 2001 From: a-maurice Date: Tue, 1 Oct 2024 16:28:26 -0700 Subject: [PATCH 2/9] Update integration_test.cc --- auth/integration_test/src/integration_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/auth/integration_test/src/integration_test.cc b/auth/integration_test/src/integration_test.cc index e026803adb..0bdad8bc9d 100644 --- a/auth/integration_test/src/integration_test.cc +++ b/auth/integration_test/src/integration_test.cc @@ -691,19 +691,19 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileNull) { EXPECT_EQ(user.photo_url(), kPhotoUrl); firebase::auth::User::UserProfile user_profile_null; user_profile_null.display_name = kDisplayName; - user_profile_null.photo_url = null; + user_profile_null.photo_url = nullptr; firebase::Future update_profile_null = user.UpdateUserProfile(user_profile_null); WaitForCompletion(user_profile_null, "UpdateUserProfileNull"); user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); - EXPECT_EQ(user.photo_url(), null); + EXPECT_EQ(user.photo_url(), nullptr); SignOut(); WaitForCompletion( auth_->SignInWithEmailAndPassword(email.c_str(), kTestPassword), "SignInWithEmailAndPassword"); user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); - EXPECT_EQ(user.photo_url(), null); + EXPECT_EQ(user.photo_url(), nullptr); DeleteUser(); } From 818a5833b3da5bbbb5089490d506c4ec3d24c5d8 Mon Sep 17 00:00:00 2001 From: a-maurice Date: Tue, 1 Oct 2024 17:38:30 -0700 Subject: [PATCH 3/9] Update integration_test.cc --- auth/integration_test/src/integration_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/auth/integration_test/src/integration_test.cc b/auth/integration_test/src/integration_test.cc index 0bdad8bc9d..3905b53f79 100644 --- a/auth/integration_test/src/integration_test.cc +++ b/auth/integration_test/src/integration_test.cc @@ -693,7 +693,7 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileNull) { user_profile_null.display_name = kDisplayName; user_profile_null.photo_url = nullptr; firebase::Future update_profile_null = user.UpdateUserProfile(user_profile_null); - WaitForCompletion(user_profile_null, "UpdateUserProfileNull"); + WaitForCompletion(update_profile_null, "UpdateUserProfileNull"); user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); EXPECT_EQ(user.photo_url(), nullptr); @@ -729,7 +729,7 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileEmpty) { user_profile_null.display_name = kDisplayName; user_profile_null.photo_url = ""; firebase::Future update_profile_null = user.UpdateUserProfile(user_profile_null); - WaitForCompletion(user_profile_null, "UpdateUserProfileEmpty"); + WaitForCompletion(update_profile_null, "UpdateUserProfileEmpty"); user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); EXPECT_EQ(user.photo_url(), ""); From 4bb9c4fff8ff1a6b6c0080a658258126dc6a5a20 Mon Sep 17 00:00:00 2001 From: a-maurice Date: Wed, 2 Oct 2024 11:36:02 -0700 Subject: [PATCH 4/9] Update integration_test.cc --- auth/integration_test/src/integration_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/auth/integration_test/src/integration_test.cc b/auth/integration_test/src/integration_test.cc index 3905b53f79..177d6ed846 100644 --- a/auth/integration_test/src/integration_test.cc +++ b/auth/integration_test/src/integration_test.cc @@ -696,14 +696,14 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileNull) { WaitForCompletion(update_profile_null, "UpdateUserProfileNull"); user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); - EXPECT_EQ(user.photo_url(), nullptr); + EXPECT_EQ(user.photo_url(), ""); SignOut(); WaitForCompletion( auth_->SignInWithEmailAndPassword(email.c_str(), kTestPassword), "SignInWithEmailAndPassword"); user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); - EXPECT_EQ(user.photo_url(), nullptr); + EXPECT_EQ(user.photo_url(), ""); DeleteUser(); } From 95fa2d03072d993416b14509914e1bc135064bb2 Mon Sep 17 00:00:00 2001 From: a-maurice Date: Wed, 2 Oct 2024 16:29:56 -0700 Subject: [PATCH 5/9] Handle empty string in profile change --- auth/integration_test/src/integration_test.cc | 22 ++++++++++--------- auth/src/android/user_android.cc | 9 ++++++-- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/auth/integration_test/src/integration_test.cc b/auth/integration_test/src/integration_test.cc index 177d6ed846..1537493c97 100644 --- a/auth/integration_test/src/integration_test.cc +++ b/auth/integration_test/src/integration_test.cc @@ -689,21 +689,22 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileNull) { user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); EXPECT_EQ(user.photo_url(), kPhotoUrl); + // Setting the entries to null should leave the old values firebase::auth::User::UserProfile user_profile_null; - user_profile_null.display_name = kDisplayName; + user_profile_null.display_name = nullptr; user_profile_null.photo_url = nullptr; firebase::Future update_profile_null = user.UpdateUserProfile(user_profile_null); WaitForCompletion(update_profile_null, "UpdateUserProfileNull"); user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); - EXPECT_EQ(user.photo_url(), ""); + EXPECT_EQ(user.photo_url(), kPhotoUrl); SignOut(); WaitForCompletion( auth_->SignInWithEmailAndPassword(email.c_str(), kTestPassword), "SignInWithEmailAndPassword"); user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); - EXPECT_EQ(user.photo_url(), ""); + EXPECT_EQ(user.photo_url(), kPhotoUrl); DeleteUser(); } @@ -725,20 +726,21 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileEmpty) { user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); EXPECT_EQ(user.photo_url(), kPhotoUrl); - firebase::auth::User::UserProfile user_profile_null; - user_profile_null.display_name = kDisplayName; - user_profile_null.photo_url = ""; - firebase::Future update_profile_null = user.UpdateUserProfile(user_profile_null); - WaitForCompletion(update_profile_null, "UpdateUserProfileEmpty"); + // Setting the fields to empty should clear it. + firebase::auth::User::UserProfile user_profile_empty; + user_profile_empty.display_name = ""; + user_profile_empty.photo_url = ""; + firebase::Future update_profile_empty = user.UpdateUserProfile(user_profile_empty); + WaitForCompletion(update_profile_empty, "UpdateUserProfileEmpty"); user = auth_->current_user(); - EXPECT_EQ(user.display_name(), kDisplayName); + EXPECT_EQ(user.display_name(), ""); EXPECT_EQ(user.photo_url(), ""); SignOut(); WaitForCompletion( auth_->SignInWithEmailAndPassword(email.c_str(), kTestPassword), "SignInWithEmailAndPassword"); user = auth_->current_user(); - EXPECT_EQ(user.display_name(), kDisplayName); + EXPECT_EQ(user.display_name(), ""); EXPECT_EQ(user.photo_url(), ""); DeleteUser(); } diff --git a/auth/src/android/user_android.cc b/auth/src/android/user_android.cc index 7fd679e8b1..4e638a85db 100644 --- a/auth/src/android/user_android.cc +++ b/auth/src/android/user_android.cc @@ -425,7 +425,10 @@ Future User::UpdateUserProfile(const UserProfile& profile) { // Extra painfully call UserProfileChangeRequest.Builder.setPhotoUri. if (error == kAuthErrorNone && profile.photo_url != nullptr) { - jobject j_uri = CharsToJniUri(env, profile.photo_url); + jobject j_uri = nullptr; + if (strlen(profile.photo_url) > 0) { + j_uri = CharsToJniUri(env, profile.photo_url); + } jobject j_builder_discard = env->CallObjectMethod( j_user_profile_builder, userprofilebuilder::GetMethodId(userprofilebuilder::kSetPhotoUri), @@ -434,7 +437,9 @@ Future User::UpdateUserProfile(const UserProfile& profile) { if (j_builder_discard) { env->DeleteLocalRef(j_builder_discard); } - env->DeleteLocalRef(j_uri); + if (j_uri) { + env->DeleteLocalRef(j_uri); + } } jobject j_user_profile_request = nullptr; From 41eb132fa0df30e02918fb7b6f883ef1f502b21a Mon Sep 17 00:00:00 2001 From: a-maurice Date: Wed, 2 Oct 2024 16:30:27 -0700 Subject: [PATCH 6/9] Formatting --- auth/integration_test/src/integration_test.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/auth/integration_test/src/integration_test.cc b/auth/integration_test/src/integration_test.cc index 1537493c97..26642fde78 100644 --- a/auth/integration_test/src/integration_test.cc +++ b/auth/integration_test/src/integration_test.cc @@ -693,7 +693,8 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileNull) { firebase::auth::User::UserProfile user_profile_null; user_profile_null.display_name = nullptr; user_profile_null.photo_url = nullptr; - firebase::Future update_profile_null = user.UpdateUserProfile(user_profile_null); + firebase::Future update_profile_null = + user.UpdateUserProfile(user_profile_null); WaitForCompletion(update_profile_null, "UpdateUserProfileNull"); user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); @@ -730,7 +731,8 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileEmpty) { firebase::auth::User::UserProfile user_profile_empty; user_profile_empty.display_name = ""; user_profile_empty.photo_url = ""; - firebase::Future update_profile_empty = user.UpdateUserProfile(user_profile_empty); + firebase::Future update_profile_empty = + user.UpdateUserProfile(user_profile_empty); WaitForCompletion(update_profile_empty, "UpdateUserProfileEmpty"); user = auth_->current_user(); EXPECT_EQ(user.display_name(), ""); From 9d5b500665b66effcc2560943f0f37592c6c791b Mon Sep 17 00:00:00 2001 From: a-maurice Date: Wed, 2 Oct 2024 17:19:36 -0700 Subject: [PATCH 7/9] Update readme.md --- release_build_files/readme.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/release_build_files/readme.md b/release_build_files/readme.md index e66d896e9e..35adfddf37 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -631,6 +631,11 @@ workflow use only during the development of your app, not for publicly shipping code. ## Release Notes +### Upcoming +- Changes + - Auth (Android): Setting photo_url to empty string clears the field, + making it consistent with the other platforms. + ### 12.3.0 - Changes - General (iOS): Update to Firebase Cocoapods version 11.2.0. From 1ab9ba43003a6060ec3a827bd8fb25a041a2cc06 Mon Sep 17 00:00:00 2001 From: a-maurice Date: Wed, 2 Oct 2024 17:23:18 -0700 Subject: [PATCH 8/9] Update readme.md --- release_build_files/readme.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/release_build_files/readme.md b/release_build_files/readme.md index 35adfddf37..9ddcdec602 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -631,10 +631,10 @@ workflow use only during the development of your app, not for publicly shipping code. ## Release Notes -### Upcoming +### Upcoming Release - Changes - - Auth (Android): Setting photo_url to empty string clears the field, - making it consistent with the other platforms. + - Auth (Android): Setting photo_url to empty string with UpdateUserProfile + clears the field, making it consistent with the other platforms. ### 12.3.0 - Changes From 3a935cb9b188046e2b36fad68ba76064529bc42a Mon Sep 17 00:00:00 2001 From: a-maurice Date: Tue, 8 Oct 2024 14:06:34 -0700 Subject: [PATCH 9/9] Update integration_test.cc --- auth/integration_test/src/integration_test.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/auth/integration_test/src/integration_test.cc b/auth/integration_test/src/integration_test.cc index 26642fde78..eef1301adc 100644 --- a/auth/integration_test/src/integration_test.cc +++ b/auth/integration_test/src/integration_test.cc @@ -649,6 +649,7 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfile) { auth_->CreateUserWithEmailAndPassword(email.c_str(), kTestPassword); WaitForCompletion(create_user, "CreateUserWithEmailAndPassword"); EXPECT_TRUE(auth_->current_user().is_valid()); + // Set some user profile properties. firebase::auth::User user = create_user.result()->user; const char kDisplayName[] = "Hello World"; @@ -661,6 +662,8 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfile) { user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); EXPECT_EQ(user.photo_url(), kPhotoUrl); + + // Validate that the new properties are present after signing out and in. SignOut(); WaitForCompletion( auth_->SignInWithEmailAndPassword(email.c_str(), kTestPassword), @@ -677,6 +680,7 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileNull) { auth_->CreateUserWithEmailAndPassword(email.c_str(), kTestPassword); WaitForCompletion(create_user, "CreateUserWithEmailAndPassword"); EXPECT_TRUE(auth_->current_user().is_valid()); + // Set some user profile properties. firebase::auth::User user = create_user.result()->user; const char kDisplayName[] = "Hello World"; @@ -689,6 +693,7 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileNull) { user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); EXPECT_EQ(user.photo_url(), kPhotoUrl); + // Setting the entries to null should leave the old values firebase::auth::User::UserProfile user_profile_null; user_profile_null.display_name = nullptr; @@ -699,6 +704,8 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileNull) { user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); EXPECT_EQ(user.photo_url(), kPhotoUrl); + + // Validate that the new properties are present after signing out and in. SignOut(); WaitForCompletion( auth_->SignInWithEmailAndPassword(email.c_str(), kTestPassword), @@ -715,6 +722,7 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileEmpty) { auth_->CreateUserWithEmailAndPassword(email.c_str(), kTestPassword); WaitForCompletion(create_user, "CreateUserWithEmailAndPassword"); EXPECT_TRUE(auth_->current_user().is_valid()); + // Set some user profile properties. firebase::auth::User user = create_user.result()->user; const char kDisplayName[] = "Hello World"; @@ -727,6 +735,7 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileEmpty) { user = auth_->current_user(); EXPECT_EQ(user.display_name(), kDisplayName); EXPECT_EQ(user.photo_url(), kPhotoUrl); + // Setting the fields to empty should clear it. firebase::auth::User::UserProfile user_profile_empty; user_profile_empty.display_name = ""; @@ -737,6 +746,8 @@ TEST_F(FirebaseAuthTest, TestUpdateUserProfileEmpty) { user = auth_->current_user(); EXPECT_EQ(user.display_name(), ""); EXPECT_EQ(user.photo_url(), ""); + + // Validate that the properties are cleared out after signing out and in. SignOut(); WaitForCompletion( auth_->SignInWithEmailAndPassword(email.c_str(), kTestPassword),