diff --git a/docs/TESTING_UNIT.md b/docs/TESTING_UNIT.md index 94479d0..18adea7 100644 --- a/docs/TESTING_UNIT.md +++ b/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 | diff --git a/openmapview/src/main/kotlin/de/afarber/openmapview/UrlTileProvider.kt b/openmapview/src/main/kotlin/de/afarber/openmapview/UrlTileProvider.kt index d897d4e..9bbb070 100644 --- a/openmapview/src/main/kotlin/de/afarber/openmapview/UrlTileProvider.kt +++ b/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 } } diff --git a/openmapview/src/test/kotlin/de/afarber/openmapview/PredefinedTileProvidersTest.kt b/openmapview/src/test/kotlin/de/afarber/openmapview/PredefinedTileProvidersTest.kt index ee2ff21..eb2752b 100644 --- a/openmapview/src/test/kotlin/de/afarber/openmapview/PredefinedTileProvidersTest.kt +++ b/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 } } diff --git a/openmapview/src/test/kotlin/de/afarber/openmapview/UrlTileProviderTest.kt b/openmapview/src/test/kotlin/de/afarber/openmapview/UrlTileProviderTest.kt index 5648bb5..30282bb 100644 --- a/openmapview/src/test/kotlin/de/afarber/openmapview/UrlTileProviderTest.kt +++ b/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")