Send deviceid with formList requests

androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt

@@ -91,3 +91,8 @@ fun Uri.getFileName(context: Context): String? {
 
     return fileName
 }
+
+fun Uri.addQueryParam(key: String, value: String?): Uri =
+    buildUpon()
+        .appendQueryParameter(key, value)
+        .build()

androidshared/src/test/java/org/odk/collect/androidshared/system/UriExtTest.kt

@@ -1,6 +1,7 @@
 package org.odk.collect.androidshared.system
 
 import android.app.Application
+import android.net.Uri
 import androidx.core.net.toUri
 import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
@@ -41,4 +42,36 @@ class UriExtTest {
 
         assertThat(fileUri.getFileName(context), equalTo(file.name))
     }
+
+    @Test
+    fun `#addQueryParam adds single query param`() {
+        val uri = Uri.parse("https://example.com")
+        val result = uri.addQueryParam("id", "123")
+
+        assertThat(result.toString(), equalTo("https://example.com?id=123"))
+    }
+
+    @Test
+    fun `#addQueryParam preserves existing query parameters`() {
+        val uri = Uri.parse("https://example.com?foo=bar")
+        val result = uri.addQueryParam("id", "123")
+
+        assertThat(result.toString(), equalTo("https://example.com?foo=bar&id=123"))
+    }
+
+    @Test
+    fun `#addQueryParam allows null value`() {
+        val uri = Uri.parse("https://example.com")
+        val result = uri.addQueryParam("id", null)
+
+        assertThat(result.toString(), equalTo("https://example.com?id=null"))
+    }
+
+    @Test
+    fun `#addQueryParam does not change uri path`() {
+        val uri = Uri.parse("https://example.com/path/subpath")
+        val result = uri.addQueryParam("id", "123")
+
+        assertThat(result.toString(), equalTo("https://example.com/path/subpath?id=123"))
+    }
 }

collect_app/src/androidTest/java/org/odk/collect/android/support/StubOpenRosaServer.kt

@@ -270,12 +270,13 @@ class StubOpenRosaServer : OpenRosaHttpInterface {
         }
 
     private fun getFormResponse(uri: URI): InputStream {
-        val formID = uri.query.split("=".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray()[1]
+        val formID = getFormId(uri)
         return getFormXML(formID)
     }
 
     private fun getManifestResponse(uri: URI): InputStream? {
-        val formID = uri.query.split("=".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray()[1]
+        val formID = getFormId(uri)
+
         val xformItem = forms[formID.toInt()]
 
         if (xformItem.mediaFiles.isEmpty()) {
@@ -391,6 +392,11 @@ class StubOpenRosaServer : OpenRosaHttpInterface {
         )
     }
 
+    private fun getFormId(uri: URI) =
+        uri.query.split("&")
+            .first { it.startsWith("formId=") }
+            .substringAfter("=")
+
     fun deleteEntity(id: String) {
         deletedEntities.add(id)
     }

collect_app/src/main/java/org/odk/collect/android/dao/CursorLoaderFactory.java

@@ -8,6 +8,7 @@
 import org.odk.collect.android.database.instances.DatabaseInstanceColumns;
 import org.odk.collect.android.external.InstancesContract;
 import org.odk.collect.android.projects.ProjectsDataService;
+import org.odk.collect.androidshared.system.UriExtKt;
 import org.odk.collect.forms.instances.Instance;
 
 @Deprecated
@@ -123,16 +124,10 @@ private CursorLoader getInstancesCursorLoader(String selection, String[] selecti
 
         return new CursorLoader(
                 Collect.getInstance(),
-                getUriWithAnalyticsParam(uri),
+                UriExtKt.addQueryParam(uri, INTERNAL_QUERY_PARAM, "true"),
                 null,
                 selection,
                 selectionArgs,
                 sortOrder);
     }
-
-    private Uri getUriWithAnalyticsParam(Uri uri) {
-        return uri.buildUpon()
-                .appendQueryParameter(INTERNAL_QUERY_PARAM, "true")
-                .build();
-    }
 }

collect_app/src/main/java/org/odk/collect/android/formmanagement/OpenRosaClientProvider.kt

@@ -1,6 +1,8 @@
 package org.odk.collect.android.formmanagement
 
 import org.odk.collect.android.utilities.WebCredentialsUtils
+import org.odk.collect.metadata.PropertyManager
+import org.odk.collect.metadata.PropertyManager.Companion.PROPMGR_DEVICE_ID
 import org.odk.collect.openrosa.forms.OpenRosaClient
 import org.odk.collect.openrosa.http.OpenRosaHttpInterface
 import org.odk.collect.openrosa.parse.Kxml2OpenRosaResponseParser
@@ -10,18 +12,21 @@ import org.odk.collect.shared.settings.Settings
 
 class OpenRosaClientProvider(
     private val settingsFactory: ProjectDependencyFactory<Settings>,
-    private val openRosaHttpInterface: OpenRosaHttpInterface
+    private val openRosaHttpInterface: OpenRosaHttpInterface,
+    private val propertyManager: PropertyManager
 ) {
 
     fun create(projectId: String): OpenRosaClient {
         val settings = settingsFactory.create(projectId)
         val serverURL = settings.getString(ProjectKeys.KEY_SERVER_URL)!!
+        val deviceId = propertyManager.getSingularProperty(PROPMGR_DEVICE_ID)
 
         return OpenRosaClient(
             serverURL,
             openRosaHttpInterface,
             WebCredentialsUtils(settings),
-            Kxml2OpenRosaResponseParser
+            Kxml2OpenRosaResponseParser,
+            deviceId
         )
     }
 }

collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java

@@ -486,8 +486,8 @@ public AnalyticsInitializer providesAnalyticsInitializer(Analytics analytics, Ve
     }
 
     @Provides
-    public OpenRosaClientProvider providesFormSourceProvider(SettingsProvider settingsProvider, OpenRosaHttpInterface openRosaHttpInterface) {
-        return new OpenRosaClientProvider(settingsProvider::getUnprotectedSettings, openRosaHttpInterface);
+    public OpenRosaClientProvider providesFormSourceProvider(SettingsProvider settingsProvider, OpenRosaHttpInterface openRosaHttpInterface, PropertyManager propertyManager) {
+        return new OpenRosaClientProvider(settingsProvider::getUnprotectedSettings, openRosaHttpInterface, propertyManager);
     }
 
     @Provides

collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/OpenRosaServerInstanceUploader.kt

@@ -10,16 +10,14 @@ import org.odk.collect.settings.keys.ProjectKeys
 import org.odk.collect.shared.settings.Settings
 import timber.log.Timber
 import java.io.File
-import java.io.UnsupportedEncodingException
 import java.net.URI
-import java.net.URLEncoder
 import androidx.core.net.toUri
 import org.odk.collect.analytics.Analytics
 import org.odk.collect.android.analytics.AnalyticsEvents
-import org.odk.collect.android.analytics.AnalyticsUtils
 import org.odk.collect.android.application.Collect
 import org.odk.collect.android.projects.ProjectDependencyModule
 import org.odk.collect.android.utilities.ResponseMessageParser
+import org.odk.collect.entities.javarosa.parse.toUriWithParam
 import org.odk.collect.openrosa.http.CaseInsensitiveHeaders
 import org.odk.collect.openrosa.http.HttpHeadResult
 import org.odk.collect.projects.ProjectDependencyFactory
@@ -253,12 +251,7 @@ class OpenRosaServerInstanceUploader(
             else -> getServerSubmissionURL(unprotectedSettings)
         }
 
-        return try {
-            "$urlString?deviceID=" + URLEncoder.encode(deviceId ?: "", "UTF-8")
-        } catch (e: UnsupportedEncodingException) {
-            Timber.i(e, "Error encoding URL for device id : %s", deviceId)
-            urlString
-        }
+        return urlString.toUriWithParam("deviceID", deviceId).toString()
     }
 
     private fun getServerSubmissionURL(unprotectedSettings: Settings): String {

entities/src/main/java/org/odk/collect/entities/javarosa/parse/StringExt.kt

@@ -1,5 +1,7 @@
 package org.odk.collect.entities.javarosa.parse
 
+import android.net.Uri
+import androidx.core.net.toUri
 import java.util.UUID
 import kotlin.contracts.ExperimentalContracts
 import kotlin.contracts.contract
@@ -20,3 +22,9 @@ fun String?.isV4UUID(): Boolean {
         false
     }
 }
+
+fun String.toUriWithParam(key: String, value: String?): Uri =
+    this.toUri()
+        .buildUpon()
+        .appendQueryParameter(key, value)
+        .build()

entities/src/test/java/org/odk/collect/entities/javarosa/parse/StringExtTest.kt

@@ -1,10 +1,13 @@
 package org.odk.collect.entities.javarosa.parse
 
+import androidx.test.ext.junit.runners.AndroidJUnit4
 import org.hamcrest.MatcherAssert.assertThat
 import org.hamcrest.Matchers.equalTo
 import org.junit.Test
+import org.junit.runner.RunWith
 import java.util.UUID
 
+@RunWith(AndroidJUnit4::class)
 class StringExtTest {
 
     @Test
@@ -42,4 +45,36 @@ class StringExtTest {
         val invalid = "not-a-uuid"
         assertThat(invalid.isV4UUID(), equalTo(false))
     }
+
+    @Test
+    fun `#toUriWithParam adds single query param to uri`() {
+        val url = "https://example.com"
+        val result = url.toUriWithParam("id", "123")
+
+        assertThat(result.toString(), equalTo("https://example.com?id=123"))
+    }
+
+    @Test
+    fun `#toUriWithParam preserves existing query parameters`() {
+        val url = "https://example.com?foo=bar"
+        val result = url.toUriWithParam("id", "123")
+
+        assertThat(result.toString(), equalTo("https://example.com?foo=bar&id=123"))
+    }
+
+    @Test
+    fun `#toUriWithParam sets null value as query param`() {
+        val url = "https://example.com"
+        val result = url.toUriWithParam("id", null)
+
+        assertThat(result.toString(), equalTo("https://example.com?id=null"))
+    }
+
+    @Test
+    fun `#toUriWithParam does not change uri path`() {
+        val url = "https://example.com/path/subpath"
+        val result = url.toUriWithParam("id", "123")
+
+        assertThat(result.toString(), equalTo("https://example.com/path/subpath?id=123"))
+    }
 }

open-rosa/build.gradle.kts

@@ -40,9 +40,10 @@ android {
 }
 
 dependencies {
-    implementation(project(":entities"))
     coreLibraryDesugaring(libs.desugar)
 
+    implementation(project(":androidshared"))
+    implementation(project(":entities"))
     implementation(project(":shared"))
     implementation(project(":forms"))
 

open-rosa/src/main/java/org/odk/collect/openrosa/forms/OpenRosaClient.kt

@@ -1,6 +1,6 @@
 package org.odk.collect.openrosa.forms
 
-import android.net.Uri
+import org.odk.collect.entities.javarosa.parse.toUriWithParam
 import org.odk.collect.entities.server.EntitySource
 import org.odk.collect.forms.FormListItem
 import org.odk.collect.forms.FormSource
@@ -24,10 +24,11 @@ class OpenRosaClient(
     serverURL: String,
     openRosaHttpInterface: OpenRosaHttpInterface?,
     private val webCredentialsProvider: WebCredentialsProvider,
-    private val openRosaResponseParser: OpenRosaResponseParser
+    private val openRosaResponseParser: OpenRosaResponseParser,
+    deviceId: String
 ) : FormSource, EntitySource {
     private val openRosaXMLFetcher =
-        OpenRosaXmlFetcher(openRosaHttpInterface, this.webCredentialsProvider)
+        OpenRosaXmlFetcher(openRosaHttpInterface, this.webCredentialsProvider, deviceId)
 
     private var serverUrl: String = serverURL
 
@@ -153,10 +154,7 @@ class OpenRosaClient(
     }
 
     override fun fetchDeletedStates(integrityUrl: String, ids: List<String>): List<Pair<String, Boolean>> {
-        val uri = Uri.parse(integrityUrl)
-            .buildUpon()
-            .appendQueryParameter("id", ids.joinToString(","))
-            .build()
+        val uri = integrityUrl.toUriWithParam("id", ids.joinToString(","))
 
         val result = openRosaXMLFetcher.getXML(uri.toString())
         if (!result.isOpenRosaResponse) {

open-rosa/src/main/java/org/odk/collect/openrosa/forms/OpenRosaXmlFetcher.java

@@ -5,6 +5,7 @@
 
 import org.javarosa.xform.parse.XFormParser;
 import org.kxml2.kdom.Document;
+import org.odk.collect.entities.javarosa.parse.StringExtKt;
 import org.odk.collect.openrosa.http.HttpCredentialsInterface;
 import org.odk.collect.openrosa.http.HttpGetResult;
 import org.odk.collect.openrosa.http.OpenRosaHttpInterface;
@@ -28,11 +29,13 @@ public class OpenRosaXmlFetcher {
     private static final String HTTP_CONTENT_TYPE_TEXT_XML = "text/xml";
 
     private final OpenRosaHttpInterface httpInterface;
-    private WebCredentialsProvider webCredentialsUtils;
+    private WebCredentialsProvider webCredentialsProvider;
+    private final String deviceId;
 
-    OpenRosaXmlFetcher(OpenRosaHttpInterface httpInterface, WebCredentialsProvider webCredentialsUtils) {
+    OpenRosaXmlFetcher(OpenRosaHttpInterface httpInterface, WebCredentialsProvider webCredentialsProvider, String deviceId) {
         this.httpInterface = httpInterface;
-        this.webCredentialsUtils = webCredentialsUtils;
+        this.webCredentialsProvider = webCredentialsProvider;
+        this.deviceId = deviceId;
     }
 
     /**
@@ -87,11 +90,12 @@ public HttpGetResult fetch(@NonNull String downloadUrl, @Nullable final String c
             throw new Exception("Invalid server URL (no hostname): " + downloadUrl);
         }
 
-        return httpInterface.executeGetRequest(uri, contentType, webCredentialsUtils.getCredentials(uri));
+        uri = URI.create(StringExtKt.toUriWithParam(uri.toString(), "deviceID", deviceId).toString());
+        return httpInterface.executeGetRequest(uri, contentType, webCredentialsProvider.getCredentials(uri));
     }
 
-    public void updateWebCredentialsProvider(WebCredentialsProvider webCredentialsUtils) {
-        this.webCredentialsUtils = webCredentialsUtils;
+    public void updateWebCredentialsProvider(WebCredentialsProvider webCredentialsProvider) {
+        this.webCredentialsProvider = webCredentialsProvider;
     }
 
     public interface WebCredentialsProvider {