From a8943dcbea9c822ce6ae0264536799c9f2f3880a Mon Sep 17 00:00:00 2001
From: Josh Kasten <josh@onesignal.com>
Date: Thu, 2 May 2024 19:18:46 -0400
Subject: [PATCH 1/4] add HTTP header OneSignal-Install-Id

OneSignal-Install-Id is a UUIDv4 locally generated on the device and
added to as an HTTP Header to all calls made to OneSignal's backend.
This allows the OneSignal's backend know where traffic is coming
from, no matter if the SubscriptionId or OneSignalId changes or isn't
available yet.

State for the new installId is encapsulated in a new InstallIdService
class, where it handles generating the id and persisting it. Tests
were also added to ensure the persisting behavior works as expected.
---
 .../java/com/onesignal/core/CoreModule.kt     |  3 ++
 .../core/internal/device/IInstallIdService.kt | 11 ++++++
 .../internal/device/impl/InstallIdService.kt  | 35 ++++++++++++++++++
 .../core/internal/http/impl/HttpClient.kt     |  4 +++
 .../preferences/IPreferencesService.kt        |  7 ++++
 .../internal/device/InstallIdServiceTests.kt  | 36 +++++++++++++++++++
 .../core/internal/http/HttpClientTests.kt     |  3 +-
 7 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/IInstallIdService.kt
 create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/InstallIdService.kt
 create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/InstallIdServiceTests.kt

diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt
index afac786ad6..efc06d54a4 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt
@@ -13,7 +13,9 @@ import com.onesignal.core.internal.config.impl.ConfigModelStoreListener
 import com.onesignal.core.internal.database.IDatabaseProvider
 import com.onesignal.core.internal.database.impl.DatabaseProvider
 import com.onesignal.core.internal.device.IDeviceService
+import com.onesignal.core.internal.device.IInstallIdService
 import com.onesignal.core.internal.device.impl.DeviceService
+import com.onesignal.core.internal.device.impl.InstallIdService
 import com.onesignal.core.internal.http.IHttpClient
 import com.onesignal.core.internal.http.impl.HttpClient
 import com.onesignal.core.internal.http.impl.HttpConnectionFactory
@@ -53,6 +55,7 @@ internal class CoreModule : IModule {
         builder.register<Time>().provides<ITime>()
         builder.register<DatabaseProvider>().provides<IDatabaseProvider>()
         builder.register<StartupService>().provides<StartupService>()
+        builder.register<InstallIdService>().provides<IInstallIdService>()
 
         // Params (Config)
         builder.register<ConfigModelStore>().provides<ConfigModelStore>()
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/IInstallIdService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/IInstallIdService.kt
new file mode 100644
index 0000000000..ed49d4200c
--- /dev/null
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/IInstallIdService.kt
@@ -0,0 +1,11 @@
+package com.onesignal.core.internal.device
+
+import java.util.UUID
+
+interface IInstallIdService {
+    /**
+     * WARNING: This may do disk I/O on the first call, so never call this from
+     * the main thread.
+     */
+    suspend fun getId(): UUID
+}
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/InstallIdService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/InstallIdService.kt
new file mode 100644
index 0000000000..bf58c52c43
--- /dev/null
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/InstallIdService.kt
@@ -0,0 +1,35 @@
+package com.onesignal.core.internal.device.impl
+
+import com.onesignal.core.internal.device.IInstallIdService
+import com.onesignal.core.internal.preferences.IPreferencesService
+import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys
+import com.onesignal.core.internal.preferences.PreferenceStores
+import java.util.UUID
+
+/**
+ * Manages a persistent UUIDv4, generated once when app is first opened.
+ * Value is for a HTTP header, OneSignal-Install-Id, added on all calls made
+ * to OneSignal's backend. This allows the OneSignal's backend know where
+ * traffic is coming from, no matter if the SubscriptionId or OneSignalId
+ * changes or isn't available yet.
+ */
+internal class InstallIdService(
+    private val _prefs: IPreferencesService,
+) : IInstallIdService {
+    private val currentId: UUID by lazy {
+        val idFromPrefs = _prefs.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_OS_INSTALL_ID)
+        if (idFromPrefs != null) {
+            UUID.fromString(idFromPrefs)
+        } else {
+            val newId = UUID.randomUUID()
+            _prefs.saveString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_OS_INSTALL_ID, newId.toString())
+            newId
+        }
+    }
+
+    /**
+     * WARNING: This may do disk I/O on the first call, so never call this from
+     * the main thread. Disk I/O is done inside of "currentId by lazy".
+     */
+    override suspend fun getId() = currentId
+}
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt
index 5fbbbbc78a..d846b8e9d6 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt
@@ -6,6 +6,7 @@ import com.onesignal.common.JSONUtils
 import com.onesignal.common.OneSignalUtils
 import com.onesignal.common.OneSignalWrapper
 import com.onesignal.core.internal.config.ConfigModelStore
+import com.onesignal.core.internal.device.IInstallIdService
 import com.onesignal.core.internal.http.HttpResponse
 import com.onesignal.core.internal.http.IHttpClient
 import com.onesignal.core.internal.preferences.IPreferencesService
@@ -32,6 +33,7 @@ internal class HttpClient(
     private val _prefs: IPreferencesService,
     private val _configModelStore: ConfigModelStore,
     private val _time: ITime,
+    private val _installIdService: IInstallIdService,
 ) : IHttpClient {
     /**
      * Delay making network requests until we reach this time.
@@ -149,6 +151,8 @@ internal class HttpClient(
                         con.setRequestProperty("OneSignal-Subscription-Id", subscriptionId)
                     }
 
+                    con.setRequestProperty("OneSignal-Install-Id", _installIdService.getId().toString())
+
                     if (jsonBody != null) {
                         con.doInput = true
                     }
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/IPreferencesService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/IPreferencesService.kt
index ac0265b278..23435d8977 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/IPreferencesService.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/IPreferencesService.kt
@@ -215,6 +215,13 @@ object PreferenceOneSignalKeys {
      */
     const val PREFS_OS_ETAG_PREFIX = "PREFS_OS_ETAG_PREFIX_"
 
+    /**
+     * (String) A install id, a UUIDv4 generated once when app is first opened.
+     * Value is for a HTTP header, OneSignal-Install-Id, added on all calls
+     * made to OneSignal's backend.
+     */
+    const val PREFS_OS_INSTALL_ID = "PREFS_OS_INSTALL_ID"
+
     /**
      * (String) A prefix key for retrieving the response for a given HTTP GET cache key. The cache
      * key should be appended to this prefix.
diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/InstallIdServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/InstallIdServiceTests.kt
new file mode 100644
index 0000000000..777d14e725
--- /dev/null
+++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/InstallIdServiceTests.kt
@@ -0,0 +1,36 @@
+package com.onesignal.core.internal.device
+
+import com.onesignal.core.internal.device.impl.InstallIdService
+import com.onesignal.mocks.MockPreferencesService
+import io.kotest.core.spec.style.FunSpec
+import io.kotest.matchers.shouldBe
+
+class InstallIdServiceTests : FunSpec({
+    test("2 calls result in the same value") {
+        // Given
+        val service = InstallIdService(MockPreferencesService())
+
+        // When
+        val value1 = service.getId()
+        val value2 = service.getId()
+
+        // Then
+        value1 shouldBe value2
+    }
+
+    // Real world scenario we are testing is if we cold restart the app we get
+    // the same value
+    test("reads from shared prefs") {
+        // Given
+        val sharedPrefs = MockPreferencesService()
+
+        // When
+        val service1 = InstallIdService(sharedPrefs)
+        val value1 = service1.getId()
+        val service2 = InstallIdService(sharedPrefs)
+        val value2 = service2.getId()
+
+        // Then
+        value1 shouldBe value2
+    }
+})
diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/http/HttpClientTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/http/HttpClientTests.kt
index 07ce545f61..7129bef909 100644
--- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/http/HttpClientTests.kt
+++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/http/HttpClientTests.kt
@@ -1,6 +1,7 @@
 package com.onesignal.core.internal.http
 
 import com.onesignal.common.OneSignalUtils
+import com.onesignal.core.internal.device.impl.InstallIdService
 import com.onesignal.core.internal.http.impl.HttpClient
 import com.onesignal.core.internal.time.impl.Time
 import com.onesignal.debug.LogLevel
@@ -21,7 +22,7 @@ class Mocks {
     internal val response = MockHttpConnectionFactory.MockResponse()
     internal val factory = MockHttpConnectionFactory(response)
     internal val httpClient by lazy {
-        HttpClient(factory, MockPreferencesService(), mockConfigModel, Time())
+        HttpClient(factory, MockPreferencesService(), mockConfigModel, Time(), InstallIdService(MockPreferencesService()))
     }
 }
 

From 8dca91f7319dd7f38069c1e164ccc80d26dd897e Mon Sep 17 00:00:00 2001
From: Josh Kasten <josh@onesignal.com>
Date: Thu, 2 May 2024 19:42:03 -0400
Subject: [PATCH 2/4] add logging for sent HTTP headers

The motivation for this was we recently added a new HTTP header,
OneSignal-Install-Id, and wanted to confirm it is being sent,
however this will help if we need to debug any other headers.

This also makes a small improvement to separate out the debug
logic from production logic.
---
 .../core/internal/http/impl/HttpClient.kt     | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt
index d846b8e9d6..e7909bf43b 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt
@@ -24,6 +24,7 @@ import kotlinx.coroutines.withTimeout
 import org.json.JSONObject
 import java.net.ConnectException
 import java.net.HttpURLConnection
+import java.net.URL
 import java.net.UnknownHostException
 import java.util.Scanner
 import javax.net.ssl.HttpsURLConnection
@@ -163,16 +164,14 @@ internal class HttpClient(
                         con.doOutput = true
                     }
 
+                    logHTTPSent(con.requestMethod, con.url, jsonBody, con.requestProperties)
+
                     if (jsonBody != null) {
                         val strJsonBody = JSONUtils.toUnescapedEUIDString(jsonBody)
-                        Logging.debug("HttpClient: ${method ?: "GET"} $url - $strJsonBody")
-
                         val sendBytes = strJsonBody.toByteArray(charset("UTF-8"))
                         con.setFixedLengthStreamingMode(sendBytes.size)
                         val outputStream = con.outputStream
                         outputStream.write(sendBytes)
-                    } else {
-                        Logging.debug("HttpClient: ${method ?: "GET"} $url")
                     }
 
                     if (cacheKey != null) {
@@ -289,6 +288,18 @@ internal class HttpClient(
         }
     }
 
+    private fun logHTTPSent(
+        method: String?,
+        url: URL,
+        jsonBody: JSONObject?,
+        headers: Map<String, List<String>>,
+    ) {
+        val headersStr = headers.entries.joinToString()
+        val methodStr = method ?: "GET"
+        val bodyStr = if (jsonBody != null) JSONUtils.toUnescapedEUIDString(jsonBody) else null
+        Logging.debug("HttpClient: Request Sent = $methodStr $url - Body: $bodyStr - Headers: $headersStr")
+    }
+
     companion object {
         private const val OS_API_VERSION = "1"
         private const val OS_ACCEPT_HEADER = "application/vnd.onesignal.v$OS_API_VERSION+json"

From 4a989a4d05796e2364fcb5c96534839d90b73ab3 Mon Sep 17 00:00:00 2001
From: Josh Kasten <josh@onesignal.com>
Date: Thu, 2 May 2024 19:45:11 -0400
Subject: [PATCH 3/4] Improve HTTPClient logcat consistency

Added "Got Response =" to the beginning of each line were we got a
response. This exact wording was picked since it is the same number of
characters as "Request Sent = ", this way it lines up in the logs,
making them easier to read.
---
 .../onesignal/core/internal/http/impl/HttpClient.kt  | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt
index e7909bf43b..a3827cb19b 100644
--- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt
+++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt
@@ -197,7 +197,7 @@ internal class HttpClient(
                                     PreferenceStores.ONESIGNAL,
                                     PreferenceOneSignalKeys.PREFS_OS_HTTP_CACHE_PREFIX + cacheKey,
                                 )
-                            Logging.debug("HttpClient: ${method ?: "GET"} $url - Using Cached response due to 304: " + cachedResponse)
+                            Logging.debug("HttpClient: Got Response = ${method ?: "GET"} ${con.url} - Using Cached response due to 304: " + cachedResponse)
 
                             // TODO: SHOULD RETURN OK INSTEAD OF NOT_MODIFIED TO MAKE TRANSPARENT?
                             retVal = HttpResponse(httpResponse, cachedResponse, retryAfterSeconds = retryAfter)
@@ -207,12 +207,12 @@ internal class HttpClient(
                             val scanner = Scanner(inputStream, "UTF-8")
                             val json = if (scanner.useDelimiter("\\A").hasNext()) scanner.next() else ""
                             scanner.close()
-                            Logging.debug("HttpClient: ${method ?: "GET"} $url - STATUS: $httpResponse JSON: " + json)
+                            Logging.debug("HttpClient: Got Response = ${method ?: "GET"} ${con.url} - STATUS: $httpResponse - Body: " + json)
 
                             if (cacheKey != null) {
                                 val eTag = con.getHeaderField("etag")
                                 if (eTag != null) {
-                                    Logging.debug("HttpClient: Response has etag of $eTag so caching the response.")
+                                    Logging.debug("HttpClient: Got Response = Response has etag of $eTag so caching the response.")
 
                                     _prefs.saveString(
                                         PreferenceStores.ONESIGNAL,
@@ -230,7 +230,7 @@ internal class HttpClient(
                             retVal = HttpResponse(httpResponse, json, retryAfterSeconds = retryAfter)
                         }
                         else -> {
-                            Logging.debug("HttpClient: ${method ?: "GET"} $url - FAILED STATUS: $httpResponse")
+                            Logging.debug("HttpClient: Got Response = ${method ?: "GET"} ${con.url} - FAILED STATUS: $httpResponse")
 
                             var inputStream = con.errorStream
                             if (inputStream == null) {
@@ -243,9 +243,9 @@ internal class HttpClient(
                                 jsonResponse =
                                     if (scanner.useDelimiter("\\A").hasNext()) scanner.next() else ""
                                 scanner.close()
-                                Logging.warn("HttpClient: $method RECEIVED JSON: $jsonResponse")
+                                Logging.warn("HttpClient: Got Response = $method - STATUS: $httpResponse - Body: $jsonResponse")
                             } else {
-                                Logging.warn("HttpClient: $method HTTP Code: $httpResponse No response body!")
+                                Logging.warn("HttpClient: Got Response = $method - STATUS: $httpResponse - No response body!")
                             }
 
                             retVal = HttpResponse(httpResponse, jsonResponse, retryAfterSeconds = retryAfter)

From 37487e0f76624d19be99fb670f8528821cfeb0b1 Mon Sep 17 00:00:00 2001
From: Josh Kasten <josh@onesignal.com>
Date: Thu, 2 May 2024 20:02:42 -0400
Subject: [PATCH 4/4] add OneSignal-Install-Id test to HttpClientTest

---
 .../com/onesignal/core/internal/http/HttpClientTests.kt     | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/http/HttpClientTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/http/HttpClientTests.kt
index 7129bef909..a14039559a 100644
--- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/http/HttpClientTests.kt
+++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/http/HttpClientTests.kt
@@ -21,8 +21,9 @@ class Mocks {
     internal val mockConfigModel = MockHelper.configModelStore()
     internal val response = MockHttpConnectionFactory.MockResponse()
     internal val factory = MockHttpConnectionFactory(response)
+    internal val installIdService = InstallIdService(MockPreferencesService())
     internal val httpClient by lazy {
-        HttpClient(factory, MockPreferencesService(), mockConfigModel, Time(), InstallIdService(MockPreferencesService()))
+        HttpClient(factory, MockPreferencesService(), mockConfigModel, Time(), installIdService)
     }
 }
 
@@ -51,7 +52,7 @@ class HttpClientTests : FunSpec({
         response.throwable should beInstanceOf<TimeoutCancellationException>()
     }
 
-    test("SDKHeader is included in all requests") {
+    test("SDK Headers are included in all requests") {
         // Given
         val mocks = Mocks()
         val httpClient = mocks.httpClient
@@ -66,6 +67,7 @@ class HttpClientTests : FunSpec({
         // Then
         for (connection in mocks.factory.connections) {
             connection.getRequestProperty("SDK-Version") shouldBe "onesignal/android/${OneSignalUtils.SDK_VERSION}"
+            connection.getRequestProperty("OneSignal-Install-Id") shouldBe mocks.installIdService.getId().toString()
         }
     }