Skip to content

Commit 5445bc5

Browse files
committed
Collapse application-password handlers to their targeted writers
After the single-writer migration the full-row write skips the credential and WP_API_REST_URL columns, so the in-memory mutations and the self-insertOrUpdateSite in updateApplicationPassword, removeApplicationPassword, and clearApplicationPasswordColumns persisted nothing — the targeted writers do all the work. Drop the dead code (keeping the new-site insert in updateApplicationPassword) and remove the now-unused insertOrUpdateSite stubs from the tests.
1 parent f9888c3 commit 5445bc5

2 files changed

Lines changed: 15 additions & 56 deletions

File tree

libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt

Lines changed: 15 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,35 +1698,23 @@ open class SiteStore @Inject constructor(
16981698
private fun updateApplicationPassword(siteModel: SiteModel): OnSiteChanged {
16991699
return try {
17001700
val siteFromDB = getSiteByLocalId(siteModel.id)
1701-
// If the site doesn't exists we rely on create a new one
1702-
val siteToStore = if (siteFromDB == null) {
1703-
siteModel
1701+
if (siteFromDB == null) {
1702+
// New site: the full insert persists everything, incl. credentials via encrypt-on-insert.
1703+
OnSiteChanged(siteSqlUtils.insertOrUpdateSite(siteModel))
17041704
} else {
1705-
siteFromDB.apply {
1706-
apiRestUsernamePlain = siteModel.apiRestUsernamePlain
1707-
apiRestPasswordPlain = siteModel.apiRestPasswordPlain
1708-
apiRestUsernameEncrypted = siteModel.apiRestUsernameEncrypted
1709-
apiRestPasswordEncrypted = siteModel.apiRestPasswordEncrypted
1710-
apiRestUsernameIV = siteModel.apiRestUsernameIV
1711-
apiRestPasswordIV = siteModel.apiRestPasswordIV
1712-
wpApiRestUrl = siteModel.wpApiRestUrl
1713-
}
1714-
siteFromDB
1715-
}
1716-
val rowsAffected = siteSqlUtils.insertOrUpdateSite(siteToStore)
1717-
// The credential columns and WP_API_REST_URL are excluded from the generic update path (see
1718-
// SiteSqlUtils), so persist them through their targeted writers for an existing site.
1719-
if (siteFromDB != null) {
1705+
// Existing site: credentials + WP_API_REST_URL are excluded from the full-row write, so
1706+
// persist them on the existing row via their targeted writers (nothing else changed).
17201707
val username = siteModel.apiRestUsernamePlain
17211708
val password = siteModel.apiRestPasswordPlain
1709+
var rowsAffected = 0
17221710
if (!username.isNullOrEmpty() && !password.isNullOrEmpty()) {
1723-
siteSqlUtils.updateApplicationPasswordCredentials(siteFromDB.id, username, password)
1711+
rowsAffected = siteSqlUtils.updateApplicationPasswordCredentials(siteFromDB.id, username, password)
17241712
}
17251713
siteModel.wpApiRestUrl?.takeIf { it.isNotEmpty() }?.let {
1726-
siteSqlUtils.updateWpApiRestUrl(siteFromDB.id, it)
1714+
rowsAffected = siteSqlUtils.updateWpApiRestUrl(siteFromDB.id, it)
17271715
}
1716+
OnSiteChanged(rowsAffected)
17281717
}
1729-
OnSiteChanged(rowsAffected)
17301718
} catch (e: DuplicateSiteException) {
17311719
OnSiteChanged(SiteError(DUPLICATE_SITE))
17321720
} catch (e: Exception) {
@@ -1748,21 +1736,10 @@ open class SiteStore @Inject constructor(
17481736
if (siteFromDB == null) {
17491737
OnSiteChanged(SiteError(SiteErrorType.INVALID_SITE))
17501738
} else {
1751-
siteFromDB.apply {
1752-
apiRestUsernamePlain = ""
1753-
apiRestPasswordPlain = ""
1754-
apiRestUsernameEncrypted = ""
1755-
apiRestPasswordEncrypted = ""
1756-
apiRestUsernameIV = ""
1757-
apiRestPasswordIV = ""
1758-
wpApiRestUrl = ""
1759-
}
1760-
val rowsAffected = siteSqlUtils.insertOrUpdateSite(siteFromDB)
1761-
// The credential columns and WP_API_REST_URL are excluded from the generic update path (see
1762-
// SiteSqlUtils), so clear them explicitly now that the application password is gone.
1739+
// Credentials + WP_API_REST_URL are excluded from the full-row write, so clear them on the
1740+
// existing row via their targeted writers now that the application password is gone.
17631741
siteSqlUtils.clearApplicationPasswordCredentials(siteFromDB.id)
1764-
siteSqlUtils.clearWpApiRestUrl(siteFromDB.id)
1765-
OnSiteChanged(rowsAffected)
1742+
OnSiteChanged(siteSqlUtils.clearWpApiRestUrl(siteFromDB.id))
17661743
}
17671744
} catch (e: DuplicateSiteException) {
17681745
OnSiteChanged(SiteError(DUPLICATE_SITE))
@@ -2497,23 +2474,9 @@ open class SiteStore @Inject constructor(
24972474
return try {
24982475
val siteFromDB = getSiteByLocalId(siteModel.id)
24992476
?: return OnSiteChanged(SiteError(SiteErrorType.INVALID_SITE))
2500-
// Clear both Plain and Encrypted columns. `SiteSqlUtils.encryptAPIRestCredentials`
2501-
// short-circuits when the encrypted columns are non-empty, so clearing only the plain
2502-
// values would leave the stale ciphertext in place and `decryptAPIRestCredentials`
2503-
// would resurrect them on the next read.
2504-
siteFromDB.apply {
2505-
apiRestUsernamePlain = ""
2506-
apiRestPasswordPlain = ""
2507-
apiRestUsernameEncrypted = ""
2508-
apiRestPasswordEncrypted = ""
2509-
apiRestUsernameIV = ""
2510-
apiRestPasswordIV = ""
2511-
}
2512-
val rowsAffected = siteSqlUtils.insertOrUpdateSite(siteFromDB)
2513-
// The credential columns are excluded from the generic update path (see SiteSqlUtils); clear them
2514-
// explicitly. wpApiRestUrl is intentionally preserved here (see KDoc) — rotation reuses it.
2515-
siteSqlUtils.clearApplicationPasswordCredentials(siteFromDB.id)
2516-
OnSiteChanged(rowsAffected)
2477+
// Credentials are excluded from the full-row write, so clear them on the existing row via the
2478+
// targeted writer. wpApiRestUrl is intentionally preserved here (see KDoc) — rotation reuses it.
2479+
OnSiteChanged(siteSqlUtils.clearApplicationPasswordCredentials(siteFromDB.id))
25172480
} catch (e: DuplicateSiteException) {
25182481
OnSiteChanged(SiteError(DUPLICATE_SITE))
25192482
}

libs/fluxc/src/test/java/org/wordpress/android/fluxc/store/SiteStoreTest.kt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,6 @@ class SiteStoreTest {
599599
url = "https://selfhosted.test"
600600
}
601601
whenever(siteSqlUtils.getSitesWithLocalId(3)).thenReturn(listOf(existing))
602-
whenever(siteSqlUtils.insertOrUpdateSite(any())).thenReturn(1)
603602
val incoming = SiteModel().apply {
604603
id = 3
605604
apiRestUsernamePlain = "user"
@@ -621,8 +620,6 @@ class SiteStoreTest {
621620
wpApiRestUrl = "https://selfhosted.test/wp-json/"
622621
}
623622
whenever(siteSqlUtils.getSitesWithLocalId(4)).thenReturn(listOf(existing))
624-
whenever(siteSqlUtils.insertOrUpdateSite(any())).thenReturn(1)
625-
626623
siteStore.onAction(
627624
SiteActionBuilder.newRemoveApplicationPasswordAction(SiteModel().apply { id = 4 })
628625
)
@@ -638,7 +635,6 @@ class SiteStoreTest {
638635
url = "https://selfhosted.test"
639636
}
640637
whenever(siteSqlUtils.getSitesWithLocalId(8)).thenReturn(listOf(existing))
641-
whenever(siteSqlUtils.insertOrUpdateSite(any())).thenReturn(1)
642638
siteStore.applicationPasswordsManagerProvider = Provider { mock<ApplicationPasswordsManager>() }
643639

644640
siteStore.deleteStoredApplicationPasswordCredentials(SiteModel().apply { id = 8 })

0 commit comments

Comments
 (0)