Skip to content

Commit 78e30c3

Browse files
brandonpagebrandonpage
committed
Add error logline if apps do not override account_type. Add unit tests.
1 parent 7b21d35 commit 78e30c3

3 files changed

Lines changed: 240 additions & 17 deletions

File tree

‎libs/SalesforceSDK/src/com/salesforce/androidsdk/app/SalesforceSDKManager.kt‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1192,7 +1192,16 @@ open class SalesforceSDKManager protected constructor(
11921192
open val isHybrid = false
11931193

11941194
/** The authentication account type, which should match authenticator.xml */
1195-
val accountType = appContext.getString(account_type)
1195+
val accountType: String by lazy {
1196+
val type = appContext.getString(account_type)
1197+
if (type == "com.salesforce.androidsdk") {
1198+
// TODO: Turn this logline into an assert in 14.0
1199+
e(TAG, "No app specific account type found. To ensure users " +
1200+
"can login override the \"account_type\" value in your strings.xml.")
1201+
}
1202+
1203+
return@lazy type
1204+
}
11961205

11971206
override fun toString() =
11981207
"""

‎libs/SalesforceSDK/src/com/salesforce/androidsdk/app/SalesforceSDKUpgradeManager.java‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public class SalesforceSDKUpgradeManager {
6262

6363
private static SalesforceSDKUpgradeManager INSTANCE = null;
6464

65-
private UserManager userManager;
65+
private final UserManager userManager;
6666

6767
/**
6868
* Returns an instance of this class.
@@ -84,12 +84,7 @@ interface UserManager {
8484
}
8585

8686
public SalesforceSDKUpgradeManager() {
87-
this(new UserManager() {
88-
@Override
89-
public List<UserAccount> getAuthenticatedUsers() {
90-
return SalesforceSDKManager.getInstance().getUserAccountManager().getAuthenticatedUsers();
91-
}
92-
});
87+
this(() -> SalesforceSDKManager.getInstance().getUserAccountManager().getAuthenticatedUsers());
9388
}
9489
public SalesforceSDKUpgradeManager(UserManager userManager) {
9590
this.userManager = userManager;
@@ -320,7 +315,7 @@ private void migrateAccountType() {
320315
final String LEGACY_ACCOUNT_TYPE = "com.salesforce.androidsdk";
321316
if (SalesforceSDKManager.getInstance().getAccountType().equals(LEGACY_ACCOUNT_TYPE)) {
322317
SalesforceSDKLogger.e(TAG, "No app specific account type found. To ensure users " +
323-
"can login override the account_type value in strings.xml.");
318+
"can login override the \"account_type\" value in your strings.xml.");
324319
return;
325320
}
326321

@@ -335,7 +330,7 @@ private void migrateAccountType() {
335330
continue;
336331
}
337332

338-
// Android OS accounts immutable so we have to remove the account and add a new one.
333+
// Android OS accounts are immutable so we have to remove the account and add a new one.
339334
accountManager.removeAccountExplicitly(account);
340335
userAccountManager.createAccount(userAccount);
341336
} catch (Exception e) {

‎libs/test/SalesforceSDKTest/src/com/salesforce/androidsdk/app/SalesforceSDKUpgradeManagerTest.kt‎

Lines changed: 226 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,35 @@
11
package com.salesforce.androidsdk.app
22

3+
import android.accounts.Account
4+
import android.accounts.AccountManager
5+
import android.content.Context
6+
import android.content.SharedPreferences
37
import android.os.Bundle
48
import androidx.test.ext.junit.runners.AndroidJUnit4
59
import androidx.test.filters.SmallTest
610
import com.salesforce.androidsdk.accounts.UserAccount
11+
import com.salesforce.androidsdk.accounts.UserAccountManager
712
import com.salesforce.androidsdk.app.SalesforceSDKUpgradeManager.UserManager
813
import com.salesforce.androidsdk.config.AdminSettingsManager
914
import com.salesforce.androidsdk.config.LegacyAdminSettingsManager
1015
import com.salesforce.androidsdk.push.PushMessaging
1116
import com.salesforce.androidsdk.push.PushService
1217
import com.salesforce.androidsdk.security.KeyStoreWrapper
18+
import io.mockk.every
19+
import io.mockk.mockk
20+
import io.mockk.mockkObject
21+
import io.mockk.unmockkAll
22+
import io.mockk.verify
23+
import org.junit.After
1324
import org.junit.Assert
1425
import org.junit.Before
1526
import org.junit.Test
1627
import org.junit.runner.RunWith
1728

29+
const val LEGACY_ACCOUNT_TYPE = "com.salesforce.androidsdk"
30+
const val VERSION_SHARED_PREF = "version_info"
31+
const val ACC_MGR_KEY = "acc_mgr_version"
32+
1833
/**
1934
* Tests for SalesforceSDKUpgradeManager
2035
*/
@@ -27,11 +42,7 @@ class SalesforceSDKUpgradeManagerTest {
2742
private val user31 = buildUser("org-3", "user-3-1")
2843
private val users = mutableListOf(user11, user12, user21, user31)
2944

30-
private val userMgr = object : UserManager {
31-
override fun getAuthenticatedUsers(): MutableList<UserAccount> {
32-
return users
33-
}
34-
}
45+
private val userMgr = UserManager { users }
3546
private val upgradeMgr = SalesforceSDKUpgradeManager(userMgr)
3647
private val legacySettingsMgr = LegacyAdminSettingsManager()
3748
private val adminSettingsMgr = AdminSettingsManager()
@@ -44,6 +55,11 @@ class SalesforceSDKUpgradeManagerTest {
4455
PushMessaging.reRegistrationRequested = false
4556
}
4657

58+
@After
59+
fun tearDown() {
60+
unmockkAll()
61+
}
62+
4763
@Test
4864
fun testNoUpgrade() {
4965
// Set up (legacy) org level custom attributes
@@ -171,11 +187,11 @@ class SalesforceSDKUpgradeManagerTest {
171187
}
172188

173189
fun setVersion(version: String) {
174-
upgradeMgr.writeCurVersion("acc_mgr_version", version)
190+
upgradeMgr.writeCurVersion(ACC_MGR_KEY, version)
175191
}
176192

177193
fun getVersion(): String {
178-
return upgradeMgr.getInstalledVersion("acc_mgr_version")
194+
return upgradeMgr.getInstalledVersion(ACC_MGR_KEY)
179195
}
180196

181197
fun buildUser(orgId: String, userId: String): UserAccount {
@@ -184,4 +200,207 @@ class SalesforceSDKUpgradeManagerTest {
184200
putString("userId", userId)
185201
})
186202
}
203+
204+
@Test
205+
fun testMigrateAccountType_legacyAccountTypeStillInUse() {
206+
// Mock SalesforceSDKManager to return the legacy account type
207+
mockkObject(SalesforceSDKManager)
208+
val mockSDKManager = mockk<SalesforceSDKManager> {
209+
every { accountType } returns LEGACY_ACCOUNT_TYPE
210+
every { appContext.getSharedPreferences(any(), any()) } returns mockk(relaxed = true)
211+
}
212+
every { SalesforceSDKManager.getInstance() } returns mockSDKManager
213+
214+
// Create upgrade manager and upgrade
215+
val upgradeManager = SalesforceSDKUpgradeManager(userMgr)
216+
upgradeManager.writeCurVersion(ACC_MGR_KEY, "14.0.0")
217+
upgradeManager.upgrade()
218+
219+
// Verify that no account operations were attempted (early return)
220+
verify(exactly = 0) { mockSDKManager.clientManager }
221+
verify(exactly = 0) { mockSDKManager.userAccountManager }
222+
}
223+
224+
@Test
225+
fun testMigrateAccountType_successfulMigration() {
226+
val legacyAccount1 = Account("user1@example.com", LEGACY_ACCOUNT_TYPE)
227+
val legacyAccount2 = Account("user2@example.com", LEGACY_ACCOUNT_TYPE)
228+
val userAccount1 = buildUser("org1", "user1")
229+
val userAccount2 = buildUser("org2", "user2")
230+
val mockAccountManager = mockk<AccountManager> {
231+
every { getAccountsByType(LEGACY_ACCOUNT_TYPE) } returns arrayOf(legacyAccount1, legacyAccount2)
232+
// OS remove account mocked to true
233+
every { removeAccountExplicitly(any()) } returns true
234+
}
235+
val mockUserAccountManager = mockk<UserAccountManager> {
236+
every { buildUserAccount(legacyAccount1) } returns userAccount1
237+
every { buildUserAccount(legacyAccount2) } returns userAccount2
238+
every { createAccount(any()) } returns mockk<Bundle>()
239+
}
240+
241+
mockkObject(SalesforceSDKManager)
242+
every { SalesforceSDKManager.getInstance() } returns mockk {
243+
every { appContext.getSharedPreferences(VERSION_SHARED_PREF, Context.MODE_PRIVATE) } returns mockk {
244+
every { getString(ACC_MGR_KEY, "") } returns "14.0.0"
245+
every { edit() } returns mockk<SharedPreferences.Editor> {
246+
every { putString(any(), any()) } returns this
247+
every { commit() } returns true
248+
}
249+
}
250+
every { additionalOauthKeys } returns null
251+
every { accountType } returns "com.new.account_type"
252+
every { clientManager.accountManager } returns mockAccountManager
253+
every { userAccountManager } returns mockUserAccountManager
254+
}
255+
256+
// Create upgrade manager and trigger migration
257+
val upgradeManager = SalesforceSDKUpgradeManager(userMgr)
258+
upgradeManager.writeCurVersion(ACC_MGR_KEY, "14.0.0")
259+
upgradeManager.upgrade()
260+
261+
// Verify all accounts were processed
262+
verify { mockAccountManager.removeAccountExplicitly(legacyAccount1) }
263+
verify { mockAccountManager.removeAccountExplicitly(legacyAccount2) }
264+
verify { mockUserAccountManager.createAccount(userAccount1) }
265+
verify { mockUserAccountManager.createAccount(userAccount2) }
266+
}
267+
268+
@Test
269+
fun testMigrateAccountType_noLegacyAccounts() {
270+
val mockAccountManager = mockk<AccountManager> {
271+
every { getAccountsByType(LEGACY_ACCOUNT_TYPE) } returns emptyArray()
272+
}
273+
val mockUserAccountManager = mockk<UserAccountManager>()
274+
275+
mockkObject(SalesforceSDKManager)
276+
every { SalesforceSDKManager.getInstance() } returns mockk {
277+
every { appContext.getSharedPreferences(VERSION_SHARED_PREF, Context.MODE_PRIVATE) } returns mockk {
278+
every { getString(ACC_MGR_KEY, "") } returns "14.0.0"
279+
every { edit() } returns mockk<SharedPreferences.Editor> {
280+
every { putString(any(), any()) } returns this
281+
every { commit() } returns true
282+
}
283+
}
284+
every { additionalOauthKeys } returns null
285+
every { accountType } returns "com.new.account_type"
286+
every { clientManager.accountManager } returns mockAccountManager
287+
every { userAccountManager } returns mockUserAccountManager
288+
}
289+
290+
// Create upgrade manager and trigger migration
291+
val upgradeManager = SalesforceSDKUpgradeManager(userMgr)
292+
upgradeManager.writeCurVersion(ACC_MGR_KEY, "14.0.0")
293+
upgradeManager.upgrade()
294+
295+
// Verify no account operations were performed
296+
verify(exactly = 0) { mockAccountManager.removeAccountExplicitly(any()) }
297+
verify(exactly = 0) { mockUserAccountManager.createAccount(any()) }
298+
}
299+
300+
@Test
301+
fun testMigrateAccountType_buildUserAccountReturnsNull() {
302+
val legacyAccount1 = Account("user1@example.com", LEGACY_ACCOUNT_TYPE)
303+
val legacyAccount2 = Account("user2@example.com", LEGACY_ACCOUNT_TYPE)
304+
val userAccount2 = buildUser("org2", "user2")
305+
val mockAccountManager = mockk<AccountManager> {
306+
every { getAccountsByType(LEGACY_ACCOUNT_TYPE) } returns arrayOf(legacyAccount1, legacyAccount2)
307+
// OS remove account mocked to true
308+
every { removeAccountExplicitly(any()) } returns true
309+
}
310+
val mockUserAccountManager = mockk<UserAccountManager> {
311+
// mock corrupted account error
312+
every { buildUserAccount(legacyAccount1) } returns null
313+
every { buildUserAccount(legacyAccount2) } returns userAccount2
314+
every { createAccount(any()) } returns mockk<Bundle>()
315+
}
316+
317+
mockkObject(SalesforceSDKManager)
318+
every { SalesforceSDKManager.getInstance() } returns mockk {
319+
every { appContext.getSharedPreferences(VERSION_SHARED_PREF, Context.MODE_PRIVATE) } returns mockk {
320+
every { getString(ACC_MGR_KEY, "") } returns "14.0.0"
321+
every { edit() } returns mockk<SharedPreferences.Editor> {
322+
every { putString(any(), any()) } returns this
323+
every { commit() } returns true
324+
}
325+
}
326+
every { additionalOauthKeys } returns null
327+
every { accountType } returns "com.new.account_type"
328+
every { clientManager.accountManager } returns mockAccountManager
329+
every { userAccountManager } returns mockUserAccountManager
330+
}
331+
332+
// Create upgrade manager and trigger migration
333+
val upgradeManager = SalesforceSDKUpgradeManager(userMgr)
334+
upgradeManager.writeCurVersion(ACC_MGR_KEY, "14.0.0")
335+
upgradeManager.upgrade()
336+
337+
// Verify account was not removed or recreated when buildUserAccount returns null, but other accounts still succeed.
338+
verify(exactly = 0) { mockAccountManager.removeAccountExplicitly(legacyAccount1) }
339+
verify(exactly = 1) {
340+
mockAccountManager.removeAccountExplicitly(legacyAccount2)
341+
mockUserAccountManager.createAccount(userAccount2)
342+
mockUserAccountManager.createAccount(any())
343+
}
344+
}
345+
346+
@Test
347+
fun testMigrateAccountType_exceptionDuringMigration() {
348+
val legacyAccount1 = Account("user1@example.com", LEGACY_ACCOUNT_TYPE)
349+
val legacyAccount2 = Account("user2@example.com", LEGACY_ACCOUNT_TYPE)
350+
val userAccount1 = buildUser("org1", "user1")
351+
val userAccount2 = buildUser("org2", "user2")
352+
val mockAccountManager = mockk<AccountManager> {
353+
every { getAccountsByType(LEGACY_ACCOUNT_TYPE) } returns arrayOf(legacyAccount1, legacyAccount2)
354+
// OS remove account throws for first account
355+
every { removeAccountExplicitly(legacyAccount1) } throws RuntimeException("Remove failed")
356+
every { removeAccountExplicitly(legacyAccount2) } returns true
357+
}
358+
val mockUserAccountManager = mockk<UserAccountManager> {
359+
// mock corrupted account error
360+
every { buildUserAccount(legacyAccount1) } returns null
361+
every { buildUserAccount(legacyAccount2) } returns userAccount2
362+
every { createAccount(any()) } returns mockk<Bundle>()
363+
}
364+
365+
mockkObject(SalesforceSDKManager)
366+
every { SalesforceSDKManager.getInstance() } returns mockk {
367+
every { appContext.getSharedPreferences(VERSION_SHARED_PREF, Context.MODE_PRIVATE) } returns mockk {
368+
every { getString(ACC_MGR_KEY, "") } returns "14.0.0"
369+
every { edit() } returns mockk<SharedPreferences.Editor> {
370+
every { putString(any(), any()) } returns this
371+
every { commit() } returns true
372+
}
373+
}
374+
every { additionalOauthKeys } returns null
375+
every { accountType } returns "com.new.account_type"
376+
every { clientManager.accountManager } returns mockAccountManager
377+
every { userAccountManager } returns mockUserAccountManager
378+
}
379+
380+
// Create upgrade manager and trigger migration
381+
val upgradeManager = SalesforceSDKUpgradeManager(userMgr)
382+
upgradeManager.writeCurVersion(ACC_MGR_KEY, "14.0.0")
383+
upgradeManager.upgrade()
384+
385+
// Verify second account was still processed despite first account failing
386+
verify(exactly = 0) { mockUserAccountManager.createAccount(userAccount1) }
387+
verify(exactly = 1) {
388+
mockAccountManager.removeAccountExplicitly(legacyAccount2)
389+
mockUserAccountManager.createAccount(userAccount2)
390+
}
391+
}
392+
393+
@Test
394+
fun testMigrateAccountType_noUpgradeNeeded() {
395+
mockkObject(SalesforceSDKManager)
396+
val mockSDKManager = mockk<SalesforceSDKManager>(relaxed = true)
397+
every { SalesforceSDKManager.getInstance() } returns mockSDKManager
398+
399+
// Create upgrade manager and set version to 15.0.0 (no upgrade needed)
400+
val upgradeManager = SalesforceSDKUpgradeManager(userMgr)
401+
upgradeManager.writeCurVersion(ACC_MGR_KEY, "15.0.0")
402+
upgradeManager.upgrade()
403+
404+
verify(exactly = 0) { mockSDKManager.accountType }
405+
}
187406
}

0 commit comments

Comments
 (0)