Change p2p fetch records query to fix slowness (#2554) (#2562)

* Change p2p fetch records query to fix slowness (#2554) Fixes #2536 Co-authored-by: Benjamin Mwalimu <dubdabasoduba@gmail.com> * update test --------- Co-authored-by: Benjamin Mwalimu <dubdabasoduba@gmail.com> Co-authored-by: pld <peter@ona.io>
opensrp · Jul 10, 2023 · 54d3e95 · 54d3e95
1 parent deccb26
commit 54d3e95
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - Incorrect error shown for failed authentication when the credentials are invalid.
+- [P2P] Possible p2p sync slow regression #2536
 
 ### Changed
 - 

diff --git a/android/engine/src/main/java/org/smartregister/fhircore/engine/p2p/dao/BaseP2PTransferDao.kt b/android/engine/src/main/java/org/smartregister/fhircore/engine/p2p/dao/BaseP2PTransferDao.kt
@@ -97,15 +97,15 @@ constructor(
             SELECT a.serializedResource
               FROM ResourceEntity a
               LEFT JOIN DateIndexEntity b
-              ON a.resourceType = b.resourceType AND a.resourceUuid = b.resourceUuid 
+              ON a.resourceType = b.resourceType AND a.resourceUuid = b.resourceUuid AND b.index_name = "_lastUpdated"
               LEFT JOIN DateTimeIndexEntity c
-              ON a.resourceType = c.resourceType AND a.resourceUuid = c.resourceUuid
-              WHERE a.resourceUuid IN (
+              ON a.resourceType = c.resourceType AND a.resourceUuid = c.resourceUuid AND c.index_name = "_lastUpdated"
+              WHERE a.resourceType = "${classType.newInstance().resourceType}"
+              AND a.resourceUuid IN (
               SELECT resourceUuid FROM DateTimeIndexEntity
-              WHERE resourceType = '${classType.newInstance().resourceType}' AND index_name = '_lastUpdated' AND index_to >= ?
+              WHERE resourceType = "${classType.newInstance().resourceType}" AND index_name = "_lastUpdated" AND index_to >= ?
               )
-              AND (b.index_name = '_lastUpdated' OR c.index_name = '_lastUpdated')
-              ORDER BY c.index_from ASC, a.id ASC
+              ORDER BY b.index_from ASC, c.index_from ASC
               LIMIT ? OFFSET ?
           """.trimIndent(),
           listOf(lastRecordUpdatedAt, batchSize, offset)

diff --git a/.../engine/src/test/java/org/smartregister/fhircore/engine/p2p/dao/BaseP2PTransferDaoTest.kt b/.../engine/src/test/java/org/smartregister/fhircore/engine/p2p/dao/BaseP2PTransferDaoTest.kt
@@ -24,25 +24,17 @@ import io.mockk.every
 import io.mockk.mockk
 import io.mockk.slot
 import io.mockk.spyk
-import java.util.Date
 import java.util.TreeSet
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.test.runTest
-import org.hl7.fhir.r4.model.Address
-import org.hl7.fhir.r4.model.ContactPoint
 import org.hl7.fhir.r4.model.Encounter
-import org.hl7.fhir.r4.model.Enumerations
 import org.hl7.fhir.r4.model.Group
-import org.hl7.fhir.r4.model.HumanName
 import org.hl7.fhir.r4.model.ListResource
-import org.hl7.fhir.r4.model.Meta
 import org.hl7.fhir.r4.model.Observation
 import org.hl7.fhir.r4.model.Patient
 import org.hl7.fhir.r4.model.Questionnaire
 import org.hl7.fhir.r4.model.QuestionnaireResponse
 import org.hl7.fhir.r4.model.ResourceType
-import org.hl7.fhir.r4.model.StringType
-import org.joda.time.LocalDate
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
 import org.junit.Before
@@ -55,15 +47,10 @@ import org.smartregister.fhircore.engine.util.extension.resourceClassType
 import org.smartregister.p2p.sync.DataType
 
 class BaseP2PTransferDaoTest : RobolectricTest() {
-
   private lateinit var baseP2PTransferDao: BaseP2PTransferDao
-
   private val configurationRegistry: ConfigurationRegistry = Faker.buildTestConfigurationRegistry()
-
   private val fhirEngine: FhirEngine = mockk(relaxed = true)
 
-  private val currentDate = Date()
-
   @Before
   fun setUp() {
     baseP2PTransferDao =
@@ -79,7 +66,6 @@ class BaseP2PTransferDaoTest : RobolectricTest() {
 
   @Test
   fun `getDataTypes() returns correct list of datatypes`() {
-
     val actualDataTypes = baseP2PTransferDao.getDataTypes()
     assertEquals(9, actualDataTypes.size)
     assertTrue(
@@ -145,15 +131,15 @@ class BaseP2PTransferDaoTest : RobolectricTest() {
       "SELECT a.serializedResource\n" +
         "  FROM ResourceEntity a\n" +
         "  LEFT JOIN DateIndexEntity b\n" +
-        "  ON a.resourceType = b.resourceType AND a.resourceUuid = b.resourceUuid \n" +
+        "  ON a.resourceType = b.resourceType AND a.resourceUuid = b.resourceUuid AND b.index_name = \"_lastUpdated\"\n" +
         "  LEFT JOIN DateTimeIndexEntity c\n" +
-        "  ON a.resourceType = c.resourceType AND a.resourceUuid = c.resourceUuid\n" +
-        "  WHERE a.resourceUuid IN (\n" +
+        "  ON a.resourceType = c.resourceType AND a.resourceUuid = c.resourceUuid AND c.index_name = \"_lastUpdated\"\n" +
+        "  WHERE a.resourceType = \"Patient\"\n" +
+        "  AND a.resourceUuid IN (\n" +
         "  SELECT resourceUuid FROM DateTimeIndexEntity\n" +
-        "  WHERE resourceType = 'Patient' AND index_name = '_lastUpdated' AND index_to >= ?\n" +
+        "  WHERE resourceType = \"Patient\" AND index_name = \"_lastUpdated\" AND index_to >= ?\n" +
         "  )\n" +
-        "  AND (b.index_name = '_lastUpdated' OR c.index_name = '_lastUpdated')\n" +
-        "  ORDER BY c.index_from ASC, a.id ASC\n" +
+        "  ORDER BY b.index_from ASC, c.index_from ASC\n" +
         "  LIMIT ? OFFSET ?"
 
     val patientDataType = DataType("Patient", DataType.Filetype.JSON, 1)
@@ -166,8 +152,8 @@ class BaseP2PTransferDaoTest : RobolectricTest() {
         classType = classType
       )
     }
-
     val searchQuerySlot = slot<SearchQuery>()
+
     coVerify { fhirEngine.search<Patient>(capture(searchQuerySlot)) }
     assertEquals(25, searchQuerySlot.captured.args[1])
     assertEquals(expectedQuery, searchQuerySlot.captured.query)
@@ -252,32 +238,4 @@ class BaseP2PTransferDaoTest : RobolectricTest() {
       actualDataTypes.contains(DataType(ResourceType.Encounter.name, DataType.Filetype.JSON, 5))
     )
   }
-
-  private fun populateTestPatient(): Patient {
-    val patientId = "patient-123456"
-    val patient: Patient =
-      Patient().apply {
-        id = patientId
-        active = true
-        birthDate = LocalDate.parse("1999-10-03").toDate()
-        gender = Enumerations.AdministrativeGender.MALE
-        address =
-          listOf(
-            Address().apply {
-              city = "Nairobi"
-              country = "Kenya"
-            }
-          )
-        name =
-          listOf(
-            HumanName().apply {
-              given = mutableListOf(StringType("Kiptoo"))
-              family = "Maina"
-            }
-          )
-        telecom = listOf(ContactPoint().apply { value = "12345" })
-        meta = Meta().apply { lastUpdated = currentDate }
-      }
-    return patient
-  }
 }