Skip to content

Commit e65b78e

Browse files
committed
Stop full-row site writes from clobbering xmlRpcUrl
xmlRpcUrl is set either out of band (XML-RPC rediscovery) or by the WP.com REST sync (meta.links.xmlrpc). A full-row insertOrUpdateSite UPDATE built from a partial SiteModel that doesn't carry it — e.g. the WPAPI fetch, which builds a model with a null xmlRpcUrl — wrote that null over a good value. Preserve XMLRPC_URL on absence in the generic update path: keep the stored value when the inbound model has none, persist it when the model carries one (the WP.com sync, including a changed endpoint after a domain migration). Add a targeted updateXmlRpcUrl writer for the single-column rediscovery heal; the recovery flow that calls it lands separately. Extends the #22905 fix to XMLRPC_URL.
1 parent bc35787 commit e65b78e

2 files changed

Lines changed: 105 additions & 3 deletions

File tree

libs/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/SiteSqlUtils.kt

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,18 @@ class SiteSqlUtils
221221
AppLog.d(DB, "Updating site: " + finalSiteModel.url)
222222
val oldId = siteResult[0].id
223223
try {
224-
// WP_API_REST_URL and the application-password credential columns are written out of band by
225-
// dedicated single-column writers (updateWpApiRestUrl / updateApplicationPasswordCredentials),
226-
// so they're excluded from the generic mapper to keep stale full-row writes from clobbering them.
224+
// WP_API_REST_URL and the application-password credential columns are discovered/healed out
225+
// of band — never carried by the general site-sync responses — so they're excluded from the
226+
// generic mapper and written only by their dedicated writers (updateWpApiRestUrl /
227+
// updateApplicationPasswordCredentials); a stale full-row write must not clobber them.
228+
//
229+
// XMLRPC_URL is different: the WP.com REST sync reliably carries it (meta.links.xmlrpc), so
230+
// the generic write persists it — that's how a changed endpoint (e.g. a domain migration)
231+
// lands. But partial writers that don't carry it (the WPAPI fetch builds a model with a null
232+
// xmlRpcUrl) must not clear a stored/rediscovered value, so preserve it on absence.
233+
if (finalSiteModel.xmlRpcUrl.isNullOrEmpty()) {
234+
finalSiteModel.xmlRpcUrl = siteResult[0].xmlRpcUrl
235+
}
227236
WellSql.update(SiteModel::class.java).whereId(oldId)
228237
.put(
229238
finalSiteModel,
@@ -295,6 +304,23 @@ class SiteSqlUtils
295304
*/
296305
fun clearWpApiRestUrl(localId: Int): Int = updateWpApiRestUrl(localId, "")
297306

307+
/**
308+
* Targeted writer for [SiteModel.xmlRpcUrl], used by the XML-RPC rediscovery heal to persist just that
309+
* one column without a full-row write of an in-memory model. Unlike [updateWpApiRestUrl], XMLRPC_URL is
310+
* NOT excluded from the generic mapper — the WP.com sync reliably carries it — but the generic update
311+
* path preserves it on absence, so a partial write can't clobber a rediscovered value. (The recovery flow
312+
* that calls this lands separately.)
313+
*/
314+
fun updateXmlRpcUrl(localId: Int, xmlRpcUrl: String): Int {
315+
return WellSql.update(SiteModel::class.java)
316+
.whereId(localId)
317+
.put(xmlRpcUrl, { value ->
318+
val cv = ContentValues()
319+
cv.put(SiteModelTable.XMLRPC_URL, value)
320+
cv
321+
}).execute()
322+
}
323+
298324
/**
299325
* Targeted writer for the application-password credential columns: the encrypted username and password
300326
* and their IVs. The four are written as a set — the IVs are required to decrypt the ciphertext, so

libs/fluxc/src/test/java/org/wordpress/android/fluxc/persistence/SiteSqlUtilsTest.kt

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,82 @@ class SiteSqlUtilsTest {
191191
assertThat(stored.apiRestPasswordIV).isEmpty()
192192
}
193193

194+
@Test
195+
fun `updateXmlRpcUrl writes the column and leaves other fields alone`() {
196+
WellSql.insert(SiteModel().apply {
197+
siteId = 42
198+
url = "https://example.test"
199+
name = "Example"
200+
xmlRpcUrl = null
201+
}).execute()
202+
val localId = siteSqlUtils.getSites().single().id
203+
204+
val rowsUpdated = siteSqlUtils.updateXmlRpcUrl(localId, "https://example.test/xmlrpc.php")
205+
206+
assertThat(rowsUpdated).isEqualTo(1)
207+
val stored = siteSqlUtils.getSites().single()
208+
assertThat(stored.xmlRpcUrl).isEqualTo("https://example.test/xmlrpc.php")
209+
assertThat(stored.url).isEqualTo("https://example.test")
210+
assertThat(stored.name).isEqualTo("Example")
211+
}
212+
213+
@Test
214+
fun `updateXmlRpcUrl returns 0 when no site row matches the local id`() {
215+
val rowsUpdated = siteSqlUtils.updateXmlRpcUrl(localId = 999, xmlRpcUrl = "https://example.test/xmlrpc.php")
216+
217+
assertThat(rowsUpdated).isEqualTo(0)
218+
}
219+
220+
@Test
221+
fun `insertOrUpdateSite update does not clobber xmlRpcUrl from a stale model`() {
222+
// Absence is preserved: a partial writer that doesn't carry xmlRpcUrl (e.g. the WPAPI fetch) must
223+
// not clear a stored/rediscovered value, even though XMLRPC_URL is not excluded from the mapper.
224+
val xmlRpc = "https://example.test/xmlrpc.php"
225+
WellSql.insert(SiteModel().apply {
226+
siteId = 42
227+
url = "https://example.test"
228+
name = "Example"
229+
xmlRpcUrl = xmlRpc
230+
}).execute()
231+
val localId = siteSqlUtils.getSites().single().id
232+
233+
val stale = SiteModel().apply {
234+
id = localId
235+
siteId = 42
236+
url = "https://example.test"
237+
name = "Updated name"
238+
xmlRpcUrl = null
239+
}
240+
siteSqlUtils.insertOrUpdateSite(stale)
241+
242+
val stored = siteSqlUtils.getSites().single()
243+
assertThat(stored.xmlRpcUrl).isEqualTo(xmlRpc) // preserved
244+
assertThat(stored.name).isEqualTo("Updated name") // other columns still update
245+
}
246+
247+
@Test
248+
fun `insertOrUpdateSite update persists a changed xmlRpcUrl carried by the inbound model`() {
249+
// Presence is authoritative: the WP.com sync reliably carries meta.links.xmlrpc, so a non-empty
250+
// inbound value (e.g. a domain migration) must overwrite the stored one — only absence is preserved.
251+
WellSql.insert(SiteModel().apply {
252+
siteId = 42
253+
url = "https://example.test"
254+
xmlRpcUrl = "https://example.test/xmlrpc.php"
255+
}).execute()
256+
val localId = siteSqlUtils.getSites().single().id
257+
258+
val migrated = SiteModel().apply {
259+
id = localId
260+
siteId = 42
261+
url = "https://example.test"
262+
xmlRpcUrl = "https://migrated.test/xmlrpc.php"
263+
}
264+
siteSqlUtils.insertOrUpdateSite(migrated)
265+
266+
assertThat(siteSqlUtils.getSites().single().xmlRpcUrl)
267+
.isEqualTo("https://migrated.test/xmlrpc.php")
268+
}
269+
194270
// Raw read that bypasses SiteSqlUtils' decryptAPIRestCredentials, so tests can assert on the stored
195271
// ciphertext columns directly without invoking the AndroidKeyStore-backed EncryptionUtils.
196272
private fun storedSite(): SiteModel = WellSql.select(SiteModel::class.java).asModel.single()

0 commit comments

Comments
 (0)