chore: [ANDROSDK-2384] review dataSet download flow

core/src/androidTest/java/org/hisp/dhis/android/core/datavalue/internal/DataValueEndpointCallRealIntegrationShould.kt

@@ -85,7 +85,7 @@ class DataValueEndpointCallRealIntegrationShould : BaseRealIntegrationTest() {
         val apiDownloader: APIDownloader = APIDownloaderImpl(resourceHandler)
         val dataValueNetworkHandler = DataValueNetworkHandlerImpl(d2.httpServiceClient(), d2.coroutineAPICallExecutor())
 
-        return DataValueCall(dataValueNetworkHandler, dataValueHandler, apiDownloader)
+        return DataValueCall(dataValueNetworkHandler, dataValueHandler, apiDownloader, koin.get())
             .download(DataValueQuery(bundle))
     }
 }

core/src/main/java/org/hisp/dhis/android/core/dataset/DataSet.kt

@@ -110,6 +110,11 @@ data class DataSet(
     class Builder : DataSetBuilder()
 
     companion object {
+        internal const val DEFAULT_FUTURE_PERIODS = 1
+
+        internal fun futurePeriodsOrDefault(dataSet: DataSet): Int =
+            dataSet.openFuturePeriods() ?: DEFAULT_FUTURE_PERIODS
+
         @JvmStatic
         fun builder(): Builder = Builder()
             .access(defaultAccess())

core/src/main/java/org/hisp/dhis/android/core/datavalue/internal/DataValueCall.kt

@@ -29,21 +29,52 @@ package org.hisp.dhis.android.core.datavalue.internal
 
 import org.hisp.dhis.android.core.arch.api.executors.internal.APIDownloader
 import org.hisp.dhis.android.core.arch.call.factories.internal.QueryCall
+import org.hisp.dhis.android.core.arch.helpers.internal.UrlLengthHelper
+import org.hisp.dhis.android.core.category.internal.CategoryOptionComboStore
+import org.hisp.dhis.android.core.dataset.DataSet
 import org.hisp.dhis.android.core.datavalue.DataValue
+import org.hisp.dhis.android.core.domain.aggregated.data.internal.AggregatedDataCallBundle
 import org.koin.core.annotation.Singleton
 
 @Singleton
 internal class DataValueCall(
     private val networkHandler: DataValueNetworkHandler,
     private val handler: DataValueHandler,
     private val apiDownloader: APIDownloader,
+    private val categoryOptionComboStore: CategoryOptionComboStore,
 ) : QueryCall<DataValue, DataValueQuery> {
 
+    companion object {
+        private const val QUERY_WITHOUT_UIDS_LENGTH = (
+            "dataValueSets?fields=dataElement,period,orgUnit,categoryOptionCombo,attributeOptionCombo,value," +
+                "storedBy,created,lastUpdated,comment,followup,deleted&lastUpdated=0000-00-00T00:00:00.000" +
+                "&dataSet=&period=&orgUnit=&attributeOptionCombo=&children=true&includeDeleted=true"
+            ).length
+
+        private const val DATA_SET_UIDS = 1
+    }
+
     override suspend fun download(query: DataValueQuery): List<DataValue> {
-        return query.bundle.dataSets.mapNotNull { it.uid() }.flatMap { dataSetUid ->
+        val bundle = query.bundle
+        val availableUids = availableUidsForAttributeOptionCombos(bundle)
+
+        return bundle.dataSets.flatMap { dataSet ->
+            val attributeOptionComboUids = attributeOptionComboUids(dataSet, availableUids)
             apiDownloader.downloadListAsCoroutine(handler) {
-                networkHandler.getDataValuesForDataSet(dataSetUid, query.bundle)
+                networkHandler.getDataValuesForDataSet(dataSet.uid(), attributeOptionComboUids, bundle)
             }
         }
     }
+
+    private suspend fun attributeOptionComboUids(dataSet: DataSet, availableUids: Int): List<String> {
+        val uids = categoryOptionComboStore.getForCategoryCombo(dataSet.categoryCombo().uid())
+            .map { it.uid() }
+
+        return if (uids.size <= availableUids) uids else emptyList()
+    }
+
+    private fun availableUidsForAttributeOptionCombos(bundle: AggregatedDataCallBundle): Int {
+        return UrlLengthHelper.getHowManyUidsFitInURL(QUERY_WITHOUT_UIDS_LENGTH) -
+            DATA_SET_UIDS - bundle.periodIds.size - bundle.rootOrganisationUnitUids.size
+    }
 }

core/src/main/java/org/hisp/dhis/android/core/datavalue/internal/DataValueNetworkHandler.kt

@@ -36,7 +36,11 @@ import org.hisp.dhis.android.core.imports.internal.DataValueImportSummaryWebResp
 import org.hisp.dhis.android.core.maintenance.D2Error
 
 internal interface DataValueNetworkHandler {
-    suspend fun getDataValuesForDataSet(dataSetUid: String, bundle: AggregatedDataCallBundle): List<DataValue>
+    suspend fun getDataValuesForDataSet(
+        dataSetUid: String,
+        attributeOptionComboUids: List<String>,
+        bundle: AggregatedDataCallBundle,
+    ): List<DataValue>
     suspend fun postDataValues(dataValueSet: DataValueSet): Result<DataValueImportSummary, D2Error>
     suspend fun postDataValuesWebResponse(
         dataValueSet: DataValueSet,

core/src/main/java/org/hisp/dhis/android/core/domain/aggregated/data/internal/AggregatedDataCall.kt

@@ -37,6 +37,7 @@ import org.hisp.dhis.android.core.dataapproval.DataApproval
 import org.hisp.dhis.android.core.dataapproval.internal.DataApprovalCall
 import org.hisp.dhis.android.core.dataapproval.internal.DataApprovalQuery
 import org.hisp.dhis.android.core.dataset.DataSet
+import org.hisp.dhis.android.core.dataset.DataSet.Companion.futurePeriodsOrDefault
 import org.hisp.dhis.android.core.dataset.DataSetCompleteRegistration
 import org.hisp.dhis.android.core.dataset.internal.DataSetCompleteRegistrationCall
 import org.hisp.dhis.android.core.dataset.internal.DataSetCompleteRegistrationQuery
@@ -143,7 +144,7 @@ internal class AggregatedDataCall(
                     .dataSet(dataSet.uid())
                     .periodType(dataSet.periodType()!!)
                     .pastPeriods(bundle.key.pastPeriods)
-                    .futurePeriods(dataSet.openFuturePeriods() ?: 0)
+                    .futurePeriods(futurePeriodsOrDefault(dataSet))
                     .dataElementsHash(hashHelper.getDataSetDataElementsHash(dataSet))
                     .organisationUnitsHash(bundle.allOrganisationUnitUidsSet.hashCode())
                     .lastUpdated(syncDate)

core/src/main/java/org/hisp/dhis/android/core/domain/aggregated/data/internal/AggregatedDataCallBundleFactory.kt

@@ -28,6 +28,7 @@
 package org.hisp.dhis.android.core.domain.aggregated.data.internal
 
 import org.hisp.dhis.android.core.dataset.DataSet
+import org.hisp.dhis.android.core.dataset.DataSet.Companion.futurePeriodsOrDefault
 import org.hisp.dhis.android.core.dataset.DataSetCollectionRepository
 import org.hisp.dhis.android.core.organisationunit.OrganisationUnit
 import org.hisp.dhis.android.core.organisationunit.OrganisationUnitCollectionRepository
@@ -107,7 +108,7 @@ internal class AggregatedDataCallBundleFactory(
         organisationUnitsHash: Int,
     ): AggregatedDataCallBundleKey {
         val pastPeriods = getPastPeriods(dataSetSettings, dataSet)
-        val futurePeriods = if (dataSet.openFuturePeriods() == null) 1 else dataSet.openFuturePeriods()!!
+        val futurePeriods = futurePeriodsOrDefault(dataSet)
         val syncValue = syncValues[dataSet.uid()]
 
         return AggregatedDataCallBundleKey(

core/src/main/java/org/hisp/dhis/android/network/datavalue/DataValueDTO.kt

@@ -37,10 +37,10 @@ import org.hisp.dhis.android.network.common.dto.BaseDeletableDataObjectDTO
 internal data class DataValueDTO(
     override val deleted: Boolean?,
     val dataElement: String,
-    val period: String,
-    val orgUnit: String,
-    val categoryOptionCombo: String,
-    val attributeOptionCombo: String,
+    val period: String? = null,
+    val orgUnit: String? = null,
+    val categoryOptionCombo: String? = null,
+    val attributeOptionCombo: String? = null,
     val value: String?,
     val storedBy: String?,
     val created: String?,
@@ -50,14 +50,14 @@ internal data class DataValueDTO(
 ) : BaseDeletableDataObjectDTO {
 
     @Suppress("ComplexMethod")
-    fun toDomain(sourceDataSet: String?): DataValue {
+    fun toDomain(sourceDataSet: String?, dataValueSet: DataValueSetDTO): DataValue {
         val builder = DataValue.builder().apply {
             deleted?.let { deleted(it) }
             dataElement(dataElement)
-            period(period)
-            organisationUnit(orgUnit)
-            categoryOptionCombo(categoryOptionCombo)
-            attributeOptionCombo(attributeOptionCombo)
+            (period ?: dataValueSet.period)?.let { period(it) }
+            (orgUnit ?: dataValueSet.orgUnit)?.let { organisationUnit(it) }
+            (categoryOptionCombo ?: dataValueSet.categoryOptionCombo)?.let { categoryOptionCombo(it) }
+            (attributeOptionCombo ?: dataValueSet.attributeOptionCombo)?.let { attributeOptionCombo(it) }
             value?.let { value(it) }
             storedBy?.let { storedBy(it) }
             created?.let { created(DateUtils.DATE_FORMAT.parse(it)) }

core/src/main/java/org/hisp/dhis/android/network/datavalue/DataValueNetworkHandlerImpl.kt

@@ -51,6 +51,7 @@ internal class DataValueNetworkHandlerImpl(
 
     override suspend fun getDataValuesForDataSet(
         dataSetUid: String,
+        attributeOptionComboUids: List<String>,
         bundle: AggregatedDataCallBundle,
     ): List<DataValue> {
         val apiResponse = service.getDataValues(
@@ -59,11 +60,13 @@ internal class DataValueNetworkHandlerImpl(
             dataSetUids = dataSetUid,
             periodIds = commaSeparatedCollectionValues(bundle.periodIds),
             orgUnitUids = commaSeparatedCollectionValues(bundle.rootOrganisationUnitUids),
+            attributeOptionComboUids = attributeOptionComboUids
+                .takeIf { it.isNotEmpty() }
+                ?.let { commaSeparatedCollectionValues(it) },
             children = true,
-            paging = false,
             includeDeleted = true,
         )
-        return apiResponse.dataValues.map { it.toDomain(dataSetUid) }
+        return apiResponse.dataValues.map { it.toDomain(dataSetUid, apiResponse) }
     }
 
     override suspend fun postDataValues(dataValueSet: DataValueSet): Result<DataValueImportSummary, D2Error> {

core/src/main/java/org/hisp/dhis/android/network/datavalue/DataValueService.kt

@@ -40,8 +40,8 @@ internal class DataValueService(private val client: HttpServiceClient) {
         dataSetUids: String,
         periodIds: String,
         orgUnitUids: String,
+        attributeOptionComboUids: String?,
         children: Boolean,
-        paging: Boolean,
         includeDeleted: Boolean?,
     ): DataValueSetDTO {
         return client.get {
@@ -52,9 +52,9 @@ internal class DataValueService(private val client: HttpServiceClient) {
                 attribute("dataSet", dataSetUids)
                 attribute("period", periodIds)
                 attribute("orgUnit", orgUnitUids)
+                attribute("attributeOptionCombo", attributeOptionComboUids)
                 attribute("children", children)
                 attribute("includeDeleted", includeDeleted)
-                paging(paging)
             }
         }
     }

core/src/main/java/org/hisp/dhis/android/network/datavalue/DataValueSetDTO.kt

@@ -31,14 +31,24 @@ package org.hisp.dhis.android.network.datavalue
 import kotlinx.serialization.Serializable
 import org.hisp.dhis.android.core.datavalue.internal.DataValueSet
 
+/**
+ * The properties of a dataValueSet act as defaults for the dataValues it contains: whenever a value
+ * is constant across the whole response, the server may report it here and omit it from every
+ * dataValue. DHIS2 2.44 does it with the attributeOptionCombo as soon as the request filters by it.
+ */
 @Serializable
 internal data class DataValueSetDTO(
-    val dataSet: String?,
+    val dataSet: String? = null,
+    val period: String? = null,
+    val orgUnit: String? = null,
+    val categoryOptionCombo: String? = null,
+    val attributeOptionCombo: String? = null,
+    val completeDate: String? = null,
     val dataValues: List<DataValueDTO> = emptyList(),
 ) {
     fun toDomain(): DataValueSet {
         return DataValueSet(
-            dataValues = dataValues.map { it.toDomain(dataSet) },
+            dataValues = dataValues.map { it.toDomain(dataSet, this) },
             dataSet = dataSet,
         )
     }

core/src/test/java/org/hisp/dhis/android/core/datavalue/DataValueShould.kt

@@ -31,6 +31,7 @@ import com.google.common.truth.Truth.assertThat
 import org.hisp.dhis.android.core.common.BaseIdentifiableObject
 import org.hisp.dhis.android.core.common.CoreObjectShould
 import org.hisp.dhis.android.network.datavalue.DataValueDTO
+import org.hisp.dhis.android.network.datavalue.DataValueSetDTO
 import org.junit.Test
 
 internal class DataValueShould : CoreObjectShould<DataValueDTO>(
@@ -41,7 +42,7 @@ internal class DataValueShould : CoreObjectShould<DataValueDTO>(
     @Test
     override fun map_from_json_string() {
         val dataValueDTO = deserialize()
-        val dataValue = dataValueDTO.toDomain("lyLU2wR22tC")
+        val dataValue = dataValueDTO.toDomain("lyLU2wR22tC", DataValueSetDTO())
 
         assertThat(dataValue.dataElement()).isEqualTo("s46m5MS0hxu")
         assertThat(dataValue.period()).isEqualTo("201712")