Skip to content

Commit c2cb63b

Browse files
jkmasseljkmassel
committed
Remove dead site writes after the single-writer migration
- The cookie-nonce and React Native 404 handlers reset wpApiRestUrl in memory to force rediscovery on retry, then persisted via insertOrUpdateSite/persistSiteSafely — now a no-op for the excluded column. Drop the dead DB write (keep the in-memory reset), which also orphaned ReactNativeStore.persistSiteSafely and its injected persist function; remove those. - updateSite / createOrUpdateSites copied credentials + wpApiRestUrl from the DB onto the inbound model before the write; the mapper exclusion already preserves those columns, so drop the moot copy (editor-prefs copy stays). - Rework ReactNativeStoreWPAPITest to mock SiteSqlUtils and assert on updateWpApiRestUrl (the discover path persists there now).
1 parent 0f3fbf6 commit c2cb63b

4 files changed

Lines changed: 29 additions & 65 deletions

File tree

‎libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/CookieNonceAuthenticator.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ class CookieNonceAuthenticator @Inject constructor(
7474

7575
return if (response is WPAPIResponse.Error<*> &&
7676
response.error.volleyError?.networkResponse?.statusCode == STATUS_CODE_NOT_FOUND) {
77-
// call failed with 'not found' so clear the (failing) rest url
77+
// Reset the in-memory rest url so the retry rediscovers it. The stored value is left intact
78+
// (a 404 may be transient); WP_API_REST_URL is healed only via updateWpApiRestUrl.
7879
site.wpApiRestUrl = null
79-
(siteSqlUtils::insertOrUpdateSite)(site)
8080

8181
if (usingSavedRestUrl) {
8282
// If we did the previous call with a saved rest url, try again by making

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

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import org.wordpress.android.fluxc.network.rest.wpapi.NonceRestClient
1515
import org.wordpress.android.fluxc.network.rest.wpapi.reactnative.ReactNativeWPAPIRestClient
1616
import org.wordpress.android.fluxc.network.rest.wpcom.reactnative.ReactNativeWPComRestClient
1717
import org.wordpress.android.fluxc.persistence.SiteSqlUtils
18-
import org.wordpress.android.fluxc.persistence.SiteSqlUtils.DuplicateSiteException
1918
import org.wordpress.android.fluxc.store.ReactNativeFetchResponse.Error
2019
import org.wordpress.android.fluxc.store.ReactNativeFetchResponse.Success
2120
import org.wordpress.android.fluxc.tools.CoroutineEngine
@@ -40,7 +39,6 @@ class ReactNativeStore @VisibleForTesting constructor(
4039
private val siteSqlUtils: SiteSqlUtils,
4140
private val coroutineEngine: CoroutineEngine,
4241
private val currentTimeMillis: () -> Long = System::currentTimeMillis,
43-
private val sitePersistanceFunction: (site: SiteModel) -> Int = siteSqlUtils::insertOrUpdateSite,
4442
private val uriParser: (string: String) -> Uri = Uri::parse
4543
) {
4644
@Inject constructor(
@@ -58,7 +56,6 @@ class ReactNativeStore @VisibleForTesting constructor(
5856
siteSqlUtils,
5957
coroutineEngine,
6058
System::currentTimeMillis,
61-
siteSqlUtils::insertOrUpdateSite,
6259
Uri::parse
6360
)
6461

@@ -233,9 +230,9 @@ class ReactNativeStore @VisibleForTesting constructor(
233230
}
234231

235232
HttpURLConnection.HTTP_NOT_FOUND -> {
236-
// call failed with 'not found' so clear the (failing) rest url
233+
// Reset the in-memory rest url so the retry rediscovers it. The stored value is left
234+
// intact (a 404 may be transient); WP_API_REST_URL is healed only via updateWpApiRestUrl.
237235
site.wpApiRestUrl = null
238-
persistSiteSafely(site)
239236

240237
if (usingSavedRestUrl) {
241238
// If we did the previous call with a saved rest url, try again by making
@@ -289,8 +286,8 @@ class ReactNativeStore @VisibleForTesting constructor(
289286
is Success -> response
290287
is Error -> when (response.statusCode()) {
291288
HttpURLConnection.HTTP_NOT_FOUND -> {
289+
// Reset the in-memory rest url so the retry rediscovers it (see executeWPAPIRequest).
292290
site.wpApiRestUrl = null
293-
persistSiteSafely(site)
294291

295292
if (usingSavedRestUrl) {
296293
executeWPAPIRequest(
@@ -354,16 +351,6 @@ class ReactNativeStore @VisibleForTesting constructor(
354351
return Pair(uri.path, paramMap)
355352
}
356353

357-
private fun persistSiteSafely(site: SiteModel) {
358-
try {
359-
sitePersistanceFunction.invoke(site)
360-
} catch (e: DuplicateSiteException) {
361-
// persistance failed, which is not a big deal because it just means we may need to re-discover the
362-
// rest url later.
363-
AppLog.d(AppLog.T.DB, "Error when persisting site: $e")
364-
}
365-
}
366-
367354
private fun getNonce(site: SiteModel) = nonceRestClient.getNonce(site)
368355

369356
companion object {

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

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,21 +1668,13 @@ open class SiteStore @Inject constructor(
16681668
OnSiteChanged(SiteErrorUtils.genericToSiteError(siteModel.error))
16691669
} else {
16701670
try {
1671-
// The REST API doesn't return info about the editor(s) nor the Application Password.
1672-
// Make sure to copy current values available on the DB.
1673-
// Otherwise the apps will receive an update site without editor prefs set.
1674-
// The apps will dispatch the action to update editor(s) when necessary.
1671+
// The REST API doesn't return editor prefs, so copy the current values from the DB to avoid
1672+
// emitting an updated site without them. (Credentials and wpApiRestUrl are protected by the
1673+
// mapper exclusion in SiteSqlUtils, so they don't need copying here.)
16751674
val freshSiteFromDB = getSiteByLocalId(siteModel.id)
16761675
if (freshSiteFromDB != null) {
16771676
siteModel.mobileEditor = freshSiteFromDB.mobileEditor
16781677
siteModel.webEditor = freshSiteFromDB.webEditor
1679-
if (!freshSiteFromDB.apiRestUsernameEncrypted.isNullOrEmpty()) {
1680-
siteModel.apiRestUsernameEncrypted = freshSiteFromDB.apiRestUsernameEncrypted
1681-
siteModel.apiRestPasswordEncrypted = freshSiteFromDB.apiRestPasswordEncrypted
1682-
siteModel.apiRestUsernameIV = freshSiteFromDB.apiRestUsernameIV
1683-
siteModel.apiRestPasswordIV = freshSiteFromDB.apiRestPasswordIV
1684-
siteModel.wpApiRestUrl = freshSiteFromDB.wpApiRestUrl
1685-
}
16861678
}
16871679
OnSiteChanged(siteSqlUtils.insertOrUpdateSite(siteModel))
16881680
} catch (e: DuplicateSiteException) {
@@ -1820,21 +1812,13 @@ open class SiteStore @Inject constructor(
18201812
val updatedSites = mutableListOf<SiteModel>()
18211813
for (site in sites.sites) {
18221814
try {
1823-
// The REST API doesn't return info about the editor(s) nor the Application Password.
1824-
// Make sure to copy current values available on the DB.
1825-
// Otherwise the apps will receive an update site without editor prefs set.
1826-
// The apps will dispatch the action to update editor(s) when necessary.
1815+
// The REST API doesn't return editor prefs, so copy the current values from the DB to avoid
1816+
// emitting an updated site without them. (Credentials and wpApiRestUrl are protected by the
1817+
// mapper exclusion in SiteSqlUtils, so they don't need copying here.)
18271818
val siteFromDB = getSiteBySiteId(site.siteId)
18281819
if (siteFromDB != null) {
18291820
site.mobileEditor = siteFromDB.mobileEditor
18301821
site.webEditor = siteFromDB.webEditor
1831-
if (!siteFromDB.apiRestUsernameEncrypted.isNullOrEmpty()) {
1832-
site.apiRestUsernameEncrypted = siteFromDB.apiRestUsernameEncrypted
1833-
site.apiRestPasswordEncrypted = siteFromDB.apiRestPasswordEncrypted
1834-
site.apiRestUsernameIV = siteFromDB.apiRestUsernameIV
1835-
site.apiRestPasswordIV = siteFromDB.apiRestPasswordIV
1836-
site.wpApiRestUrl = siteFromDB.wpApiRestUrl
1837-
}
18381822
}
18391823
val localId = siteSqlUtils.insertOrUpdateSiteReturningId(site)
18401824
if (localId != 0) {

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

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ import org.mockito.kotlin.verify
1515
import org.mockito.kotlin.whenever
1616
import org.robolectric.RobolectricTestRunner
1717
import org.robolectric.annotation.Config
18-
import org.wordpress.android.fluxc.TestSiteSqlUtils
1918
import org.wordpress.android.fluxc.model.SiteModel
19+
import org.wordpress.android.fluxc.persistence.SiteSqlUtils
2020
import org.wordpress.android.fluxc.network.BaseRequest.GenericErrorType.UNKNOWN
2121
import org.wordpress.android.fluxc.network.discovery.DiscoveryWPAPIRestClient
2222
import org.wordpress.android.fluxc.network.rest.wpapi.Nonce
@@ -49,9 +49,7 @@ class ReactNativeStoreWPAPITest {
4949
private lateinit var store: ReactNativeStore
5050
private lateinit var site: SiteModel
5151

52-
private interface SitePersister : (SiteModel) -> Int
53-
54-
private lateinit var sitePersistenceMock: SitePersister
52+
private val siteSqlUtils = mock<SiteSqlUtils>()
5553

5654
@Before
5755
fun setup() {
@@ -65,16 +63,14 @@ class ReactNativeStoreWPAPITest {
6563

6664
private fun initStore(nonce: Nonce?) {
6765
whenever(nonceRestClient.getNonce(any<SiteModel>())).thenReturn(nonce)
68-
sitePersistenceMock = mock()
6966
store = ReactNativeStore(
7067
mock(),
7168
wpApiRestClient,
7269
nonceRestClient,
7370
discoveryWPAPIRestClient,
74-
TestSiteSqlUtils.siteSqlUtils,
71+
siteSqlUtils,
7572
initCoroutineEngine(),
76-
{ currentTime },
77-
sitePersistenceMock
73+
{ currentTime }
7874
)
7975
}
8076

@@ -102,9 +98,9 @@ class ReactNativeStoreWPAPITest {
10298
val actualResponse = store.executeGetRequest(site, restPathWithParams)
10399
assertEquals(callWithSuccess, actualResponse)
104100
assertEquals(restUrl, site.wpApiRestUrl, "site should be updated with rest endpoint used for successful call")
105-
inOrder(discoveryWPAPIRestClient, sitePersistenceMock, wpApiRestClient, nonceRestClient) {
101+
inOrder(discoveryWPAPIRestClient, siteSqlUtils, wpApiRestClient, nonceRestClient) {
106102
verify(discoveryWPAPIRestClient).discoverWPAPIBaseURL(site.url)
107-
verify(sitePersistenceMock)(site) // persist site after discovering wpApiRestUrl
103+
verify(siteSqlUtils).updateWpApiRestUrl(site.id, restUrl) // persist discovered wpApiRestUrl
108104
verify(nonceRestClient).requestNonce(site)
109105
verify(wpApiRestClient).getRequest(fetchUrl, nonce.value)
110106
}
@@ -134,9 +130,9 @@ class ReactNativeStoreWPAPITest {
134130
val actualResponse = store.executePostRequest(site, restPathWithParams, bodyMap)
135131
assertEquals(callWithSuccess, actualResponse)
136132
assertEquals(restUrl, site.wpApiRestUrl, "site should be updated with rest endpoint used for successful call")
137-
inOrder(discoveryWPAPIRestClient, sitePersistenceMock, wpApiRestClient, nonceRestClient) {
133+
inOrder(discoveryWPAPIRestClient, siteSqlUtils, wpApiRestClient, nonceRestClient) {
138134
verify(discoveryWPAPIRestClient).discoverWPAPIBaseURL(site.url)
139-
verify(sitePersistenceMock)(site) // persist site after discovering wpApiRestUrl
135+
verify(siteSqlUtils).updateWpApiRestUrl(site.id, restUrl) // persist discovered wpApiRestUrl
140136
verify(nonceRestClient).requestNonce(site)
141137
verify(wpApiRestClient).postRequest(postURL, nonce.value)
142138
}
@@ -154,7 +150,7 @@ class ReactNativeStoreWPAPITest {
154150
val actualResponse = store.executeGetRequest(site, restPathWithParams)
155151
assertEquals(initialResponseWithSuccess, actualResponse)
156152
verify(wpApiRestClient).getRequest(fetchUrl)
157-
verify(sitePersistenceMock, never())(any()) // no wpApiRestUrl updates, so no persistence
153+
verify(siteSqlUtils, never()).updateWpApiRestUrl(any(), any()) // no wpApiRestUrl updates, so no persistence
158154
verify(discoveryWPAPIRestClient, never()).discoverWPAPIBaseURL(any())
159155
}
160156

@@ -177,9 +173,9 @@ class ReactNativeStoreWPAPITest {
177173
val actualResponse = store.executeGetRequest(site, restPathWithParams)
178174
assertEquals(initialResponseWithSuccess, actualResponse)
179175
assertEquals(restUrl, site.wpApiRestUrl, "site should be updated with rest endpoint used for successful call")
180-
inOrder(discoveryWPAPIRestClient, sitePersistenceMock, wpApiRestClient) {
176+
inOrder(discoveryWPAPIRestClient, siteSqlUtils, wpApiRestClient) {
181177
verify(discoveryWPAPIRestClient).discoverWPAPIBaseURL(site.url)
182-
verify(sitePersistenceMock)(site) // persist site after discovering wpApiRestUrl
178+
verify(siteSqlUtils).updateWpApiRestUrl(site.id, restUrl) // persist discovered wpApiRestUrl
183179
verify(wpApiRestClient).getRequest(fetchUrl)
184180
}
185181
}
@@ -206,9 +202,9 @@ class ReactNativeStoreWPAPITest {
206202
fallbackRestUrl, site.wpApiRestUrl,
207203
"site should be updated with rest endpoint used for successful call"
208204
)
209-
inOrder(discoveryWPAPIRestClient, sitePersistenceMock, wpApiRestClient) {
205+
inOrder(discoveryWPAPIRestClient, siteSqlUtils, wpApiRestClient) {
210206
verify(discoveryWPAPIRestClient).discoverWPAPIBaseURL(site.url)
211-
verify(sitePersistenceMock)(site) // persist default endpoint after failed discovery
207+
verify(siteSqlUtils).updateWpApiRestUrl(site.id, fallbackRestUrl) // persist fallback endpoint
212208
verify(wpApiRestClient).getRequest(fetchUrl)
213209
}
214210
}
@@ -239,11 +235,10 @@ class ReactNativeStoreWPAPITest {
239235
val actualResponse = store.executeGetRequest(site, restPathWithParams)
240236
assertEquals(secondResponseWithSuccess, actualResponse)
241237
assertEquals(restUrl, site.wpApiRestUrl, "should save rest endpoint used for successful call")
242-
inOrder(discoveryWPAPIRestClient, sitePersistenceMock, wpApiRestClient) {
238+
inOrder(discoveryWPAPIRestClient, siteSqlUtils, wpApiRestClient) {
243239
verify(wpApiRestClient).getRequest(incorrectUrl)
244-
verify(sitePersistenceMock)(site) // persist site after clearing wpApiRestUrl that resulted in 404 failure
245240
verify(discoveryWPAPIRestClient).discoverWPAPIBaseURL(site.url)
246-
verify(sitePersistenceMock)(site) // persist site after discovering wpApiRestUrl
241+
verify(siteSqlUtils).updateWpApiRestUrl(site.id, restUrl) // persist discovered wpApiRestUrl
247242
verify(wpApiRestClient).getRequest(correctUrl)
248243
}
249244
}
@@ -268,11 +263,10 @@ class ReactNativeStoreWPAPITest {
268263
val actualResponse = store.executeGetRequest(site, restPathWithParams)
269264
assertEquals(responseWithNotFoundError, actualResponse)
270265
assertNull(site.wpApiRestUrl, "should not update site wpApiRestEndpoint when call fails")
271-
inOrder(discoveryWPAPIRestClient, sitePersistenceMock, wpApiRestClient) {
266+
inOrder(discoveryWPAPIRestClient, siteSqlUtils, wpApiRestClient) {
272267
verify(discoveryWPAPIRestClient).discoverWPAPIBaseURL(site.url)
273-
verify(sitePersistenceMock)(site) // persist site after discovering wpApiRestUrl
268+
verify(siteSqlUtils).updateWpApiRestUrl(site.id, restUrl) // persist discovered wpApiRestUrl
274269
verify(wpApiRestClient).getRequest(fetchUrl)
275-
verify(sitePersistenceMock)(site) // persist site after clearing wpApiRestUrl that resulted in 404 failure
276270
}
277271
}
278272

@@ -483,10 +477,9 @@ class ReactNativeStoreWPAPITest {
483477
wpApiRestClient,
484478
nonceRestClient,
485479
discoveryWPAPIRestClient,
486-
TestSiteSqlUtils.siteSqlUtils,
480+
siteSqlUtils,
487481
initCoroutineEngine(),
488482
{ currentTime },
489-
sitePersistenceMock,
490483
uriParser
491484
)
492485

0 commit comments

Comments
 (0)