fix: Adhere to same spec as proxy-client-js (#34)

Previously this library added all properties on toplevel as separate GET parameters. The proxy server expects properties to be nested under the properties key, so this fix makes sure that properties are added as properties[key] = value rather than key=value Also, updates client's context when calling update context, so that further updates takes original updates under consideration. Saves our users a manual call to update client context. fixes: #14 fixes: #13 Co-authored-by: Simon Hornby <[email protected]>
Unleash · Oct 5, 2022 · c05fcd9 · c05fcd9
1 parent e48d6bc
commit c05fcd9
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 2 deletions.
diff --git a/src/main/kotlin/io/getunleash/UnleashClient.kt b/src/main/kotlin/io/getunleash/UnleashClient.kt
@@ -94,6 +94,7 @@ class UnleashClient(
 
     override fun updateContext(context: UnleashContext): CompletableFuture<Void> {
         refreshPolicy.context = context
+        this.unleashContext = context
         return refreshPolicy.refreshAsync()
     }
 

diff --git a/src/main/kotlin/io/getunleash/polling/UnleashFetcher.kt b/src/main/kotlin/io/getunleash/polling/UnleashFetcher.kt
@@ -110,12 +110,12 @@ open class UnleashFetcher(
 
     private fun buildContextUrl(ctx: UnleashContext): HttpUrl {
         var contextUrl = proxyUrl.newBuilder().addQueryParameter("appName", ctx.appName)
-            .addQueryParameter("env", ctx.environment)
+            .addQueryParameter("environment", ctx.environment)
             .addQueryParameter("userId", ctx.userId)
             .addQueryParameter("remoteAddress", ctx.remoteAddress)
             .addQueryParameter("sessionId", ctx.sessionId)
         ctx.properties.entries.forEach {
-            contextUrl = contextUrl.addQueryParameter(it.key, it.value)
+            contextUrl = contextUrl.addQueryParameter("properties[${it.key}]", it.value)
         }
         return contextUrl.build()
     }

diff --git a/src/test/kotlin/io/getunleash/UnleashClientTest.kt b/src/test/kotlin/io/getunleash/UnleashClientTest.kt
@@ -26,6 +26,7 @@ class UnleashClientTest {
         server = MockWebServer()
         server.start()
         server.enqueue(MockResponse().setBody(TestResponses.threeToggles))
+        server.enqueue(MockResponse().setBody(TestResponses.threeToggles))
         config = UnleashConfig.newBuilder()
             .pollingMode(PollingModes.autoPoll(500) {})
             .proxyUrl(server.url("/proxy").toString())
@@ -130,6 +131,47 @@ class UnleashClientTest {
         }
     }
 
+    @Test
+    fun `Updating context causes the URL used to include new parameters`() {
+        UnleashClient.newBuilder().unleashConfig(config).unleashContext(context).build().use { client ->
+            Thread.sleep(800)
+            val newCtx = context.newBuilder().addProperty("test2", "test2value").build()
+            assertThat(client.updateContext(newCtx)).succeedsWithin(Duration.ofSeconds(2))
+            val first = server.takeRequest() // initial request
+            assertThat(first.requestUrl!!.queryParameterNames).doesNotContain("properties[test2]")
+            val second = server.takeRequest() // after updateContext
+            assertThat(second.requestUrl!!.queryParameterNames).contains("properties[test2]")
+        }
+    }
+
+    // Copied from unleash-proxy-client-response
+    @Test
+    fun `Should include context fields on request`() {
+        val context = UnleashContext.newBuilder().appName("web")
+            .environment("prod")
+            .userId("123")
+            .sessionId("456")
+            .remoteAddress("address")
+            .addProperty("property1", "property1")
+            .addProperty("property2", "property2")
+            .build()
+        UnleashClient.newBuilder().unleashConfig(config).unleashContext(context).build().use { _ ->
+            Thread.sleep(500)
+            val request = server.takeRequest()
+            val url = request.requestUrl!!
+            assertThat(url.queryParameter("userId")).isEqualTo("123")
+            assertThat(url.queryParameter("sessionId")).isEqualTo("456")
+            assertThat(url.queryParameter("userId")).isEqualTo("123")
+            assertThat(url.queryParameter("sessionId")).isEqualTo("456")
+            assertThat(url.queryParameter("remoteAddress")).isEqualTo("address")
+            assertThat(url.queryParameter("properties[property1]")).isEqualTo("property1")
+            assertThat(url.queryParameter("properties[property2]")).isEqualTo("property2")
+            assertThat(url.queryParameter("appName")).isEqualTo("web")
+            assertThat(url.queryParameter("environment")).isEqualTo("prod")
+        }
+    }
+
+
     @Test
     fun `Can configure a client that does not poll until requested to do so`() {
         val manuallyStartedConfig = config.newBuilder().pollingMode(PollingModes.manuallyStartPolling(500) {}).build()