Share one HTTP client between all UrlTileProviders

docs/TESTING_UNIT.md

@@ -52,7 +52,7 @@ The `--continue` flag ensures all tests run even if some fail, useful for gettin
 
 ## Test Structure
 
-### Current Test Coverage (456 tests across 35 test classes)
+### Current Test Coverage (458 tests across 35 test classes)
 
 | Test Class | Tests | Description |
 |------------|-------|-------------|
@@ -88,7 +88,7 @@ The `--continue` flag ensures all tests run even if some fail, useful for gettin
 | **TileSourceTest** | 7 | Tile source URL generation and attribution |
 | **TileTest** | 6 | Tile content equality, validation and toString |
 | **UiSettingsTest** | 25 | UI settings (gestures, zoom controls, not-implemented features) |
-| **UrlTileProviderTest** | 2 | Overlay tile downloading, HTTP 404 handling |
+| **UrlTileProviderTest** | 4 | Overlay tile downloading, HTTP 404 handling, shared HTTP client |
 | **ViewportCalculatorTest** | 10 | Visible tile calculation |
 | **VisibleRegionTest** | 4 | Visible region bounds and corners |
 

openmapview/src/main/kotlin/de/afarber/openmapview/UrlTileProvider.kt

@@ -38,6 +38,9 @@ import kotlinx.coroutines.CancellationException
  * }
  * ```
  *
+ * All instances share one HTTP client, so creating many providers (for example when
+ * overlays are switched on and off) does not create more connection pools.
+ *
  * @property urlTemplate URL pattern with {z}, {x}, {y} placeholders
  *
  * @see TileProvider
@@ -48,15 +51,19 @@ abstract class UrlTileProvider(
 ) : TileProvider {
     companion object {
         private val TAG = UrlTileProvider::class.java.simpleName
-    }
 
-    private val client =
-        HttpClient(Android) {
-            engine {
-                connectTimeout = 10_000
-                socketTimeout = 10_000
+        // Shared by all providers and never closed, it lives as long as the process
+        private val sharedClient by lazy {
+            HttpClient(Android) {
+                engine {
+                    connectTimeout = 10_000
+                    socketTimeout = 10_000
+                }
             }
         }
+    }
+
+    private val client: HttpClient = sharedClient
 
     /**
      * Returns the User-Agent header to use for HTTP requests.
@@ -132,10 +139,11 @@ abstract class UrlTileProvider(
         }
 
     /**
-     * Closes the HTTP client and releases resources.
-     * Should be called when the tile provider is no longer needed.
+     * Does nothing. All providers share one HTTP client, which must stay open
+     * for the other providers.
      */
+    @Deprecated("UrlTileProvider instances share one HTTP client now, there is nothing to close")
     fun close() {
-        client.close()
+        // Nothing to release per instance
     }
 }

openmapview/src/test/kotlin/de/afarber/openmapview/PredefinedTileProvidersTest.kt

@@ -57,7 +57,6 @@ class PredefinedTileProvidersTest {
         reflectionField.set(provider, HttpClient(engine))
 
         provider.getTile(1, 2, 3)
-        provider.close()
         return requested
     }
 }

openmapview/src/test/kotlin/de/afarber/openmapview/UrlTileProviderTest.kt

@@ -12,10 +12,13 @@ import io.ktor.client.engine.mock.MockEngine
 import io.ktor.client.engine.mock.respond
 import io.ktor.http.HttpStatusCode
 import io.ktor.utils.io.ByteReadChannel
+import kotlinx.coroutines.isActive
 import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
+import org.junit.Assert.assertSame
+import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
@@ -54,6 +57,31 @@ class UrlTileProviderTest {
             assertNull(provider.getTile(1, 2, 3))
         }
 
+    @Test
+    fun testProvidersShareOneClient() {
+        val provider1 = object : UrlTileProvider("https://example.com/a/{z}/{x}/{y}.png") {}
+        val provider2 = PredefinedTileProviders.openSeaMap() as UrlTileProvider
+
+        assertSame(clientOf(provider1), clientOf(provider2))
+    }
+
+    @Test
+    @Suppress("DEPRECATION")
+    fun testCloseKeepsSharedClientOpen() {
+        val provider1 = object : UrlTileProvider("https://example.com/a/{z}/{x}/{y}.png") {}
+        val provider2 = object : UrlTileProvider("https://example.com/b/{z}/{x}/{y}.png") {}
+
+        provider1.close()
+
+        assertTrue(clientOf(provider2).isActive)
+    }
+
+    private fun clientOf(provider: UrlTileProvider): HttpClient {
+        val reflectionField = UrlTileProvider::class.java.getDeclaredField("client")
+        reflectionField.isAccessible = true
+        return reflectionField.get(provider) as HttpClient
+    }
+
     private fun createProvider(engine: MockEngine): UrlTileProvider {
         val provider = object : UrlTileProvider("https://example.com/{z}/{x}/{y}.png") {}
         val reflectionField = UrlTileProvider::class.java.getDeclaredField("client")