fixup! feat: implement a request cache for mutation and insertion que…

…ries #137
GenSpectrum · Mar 1, 2024 · e9781d4 · e9781d4
1 parent 9d51191
commit e9781d4
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 18 deletions.
diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloClient.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloClient.kt
@@ -24,15 +24,32 @@ const val SILO_RESPONSE_MAX_LOG_LENGTH = 10_000
 
 @Component
 class SiloClient(
+    private val cachedSiloClient: CachedSiloClient,
+    private val dataVersion: DataVersion,
+) {
+    fun <ResponseType> sendQuery(query: SiloQuery<ResponseType>): ResponseType {
+        val result = cachedSiloClient.sendQuery(query)
+        dataVersion.dataVersion = result.dataVersion
+        return result.queryResult
+    }
+
+    fun callInfo(): InfoData {
+        val info = cachedSiloClient.callInfo()
+        dataVersion.dataVersion = info.dataVersion
+        return info
+    }
+}
+
+@Component
+class CachedSiloClient(
     @Value("\${silo.url}") private val siloUrl: String,
     private val objectMapper: ObjectMapper,
-    private val dataVersion: DataVersion,
     private val requestIdContext: RequestIdContext,
 ) {
     private val httpClient = HttpClient.newHttpClient()
 
     @Cacheable("siloQueryCache", condition = "#query.action.cacheable")
-    fun <ResponseType> sendQuery(query: SiloQuery<ResponseType>): ResponseType {
+    fun <ResponseType> sendQuery(query: SiloQuery<ResponseType>): WithDataVersion<ResponseType> {
         val queryJson = objectMapper.writeValueAsString(query)
 
         log.info { "Calling SILO: $queryJson" }
@@ -43,7 +60,10 @@ class SiloClient(
         }
 
         try {
-            return objectMapper.readValue(response.body(), query.action.typeReference).queryResult
+            return WithDataVersion(
+                queryResult = objectMapper.readValue(response.body(), query.action.typeReference).queryResult,
+                dataVersion = getDataVersion(response),
+            )
         } catch (exception: Exception) {
             val message = "Could not parse response from silo: " + exception::class.toString() + " " + exception.message
             throw RuntimeException(message, exception)
@@ -114,7 +134,6 @@ class SiloClient(
             )
         }
 
-        addDataVersionToRequestScope(response)
         return response
     }
 
@@ -126,10 +145,6 @@ class SiloClient(
             null
         }
 
-    private fun addDataVersionToRequestScope(response: HttpResponse<String>) {
-        dataVersion.dataVersion = getDataVersion(response)
-    }
-
     private fun getDataVersion(response: HttpResponse<String>): String {
         return response.headers().firstValue("data-version").orElse("")
     }
@@ -143,4 +158,9 @@ data class SiloQueryResponse<ResponseType>(
     val queryResult: ResponseType,
 )
 
+data class WithDataVersion<ResponseType>(
+    val dataVersion: String,
+    val queryResult: ResponseType,
+)
+
 data class SiloErrorResponse(val error: String, val message: String)
diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloClientTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloClientTest.kt
@@ -20,7 +20,6 @@ import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
 import org.junit.jupiter.params.ParameterizedTest
-import org.junit.jupiter.params.provider.Arguments
 import org.junit.jupiter.params.provider.MethodSource
 import org.mockserver.client.MockServerClient
 import org.mockserver.integration.ClientAndServer
@@ -40,6 +39,7 @@ private const val REQUEST_ID_VALUE = "someRequestId"
 class SiloClientTest(
     @Autowired private val underTest: SiloClient,
     @Autowired private val requestIdContext: RequestIdContext,
+    @Autowired private val dataVersion: DataVersion,
 ) {
     private lateinit var mockServer: ClientAndServer
 
@@ -391,7 +391,33 @@ class SiloClientTest(
         assertThat(result1, `is`(result2))
     }
 
-    private fun expectQueryRequestAndRespondWith(httpResponse: HttpResponse, times: Times = Times.unlimited()) =
+    @Test
+    fun `GIVEN an action that should be cached WHEN I send request twice THEN data version is populated`() {
+        val dataVersionValue = "someDataVersion"
+
+        expectQueryRequestAndRespondWith(
+            response()
+                .withStatusCode(200)
+                .withHeader("data-version", dataVersionValue)
+                .withBody("""{"queryResult": []}"""),
+            Times.once(),
+        )
+
+        val query = queriesThatShouldBeCached[0]
+
+        assertThat(dataVersion.dataVersion, `is`(nullValue()))
+        underTest.sendQuery(query)
+        assertThat(dataVersion.dataVersion, `is`(dataVersionValue))
+
+        dataVersion.dataVersion = null
+        underTest.sendQuery(query)
+        assertThat(dataVersion.dataVersion, `is`(dataVersionValue))
+    }
+
+    private fun expectQueryRequestAndRespondWith(
+        httpResponse: HttpResponse,
+        times: Times = Times.unlimited(),
+    ) {
         MockServerClient("localhost", MOCK_SERVER_PORT)
             .`when`(
                 request()
@@ -402,22 +428,23 @@ class SiloClientTest(
                 times,
             )
             .respond(httpResponse)
+    }
 
     companion object {
         @JvmStatic
         val queriesThatShouldNotBeCached = listOf(
-            Arguments.of(SiloQuery(SiloAction.aggregated(), True)),
-            Arguments.of(SiloQuery(SiloAction.details(), True)),
-            Arguments.of(SiloQuery(SiloAction.genomicSequence(SequenceType.ALIGNED, "sequenceName"), True)),
-            Arguments.of(SiloQuery(SiloAction.genomicSequence(SequenceType.UNALIGNED, "sequenceName"), True)),
+            SiloQuery(SiloAction.aggregated(), True),
+            SiloQuery(SiloAction.details(), True),
+            SiloQuery(SiloAction.genomicSequence(SequenceType.ALIGNED, "sequenceName"), True),
+            SiloQuery(SiloAction.genomicSequence(SequenceType.UNALIGNED, "sequenceName"), True),
         )
 
         @JvmStatic
         val queriesThatShouldBeCached = listOf(
-            Arguments.of(SiloQuery(SiloAction.mutations(), True)),
-            Arguments.of(SiloQuery(SiloAction.aminoAcidMutations(), True)),
-            Arguments.of(SiloQuery(SiloAction.nucleotideInsertions(), True)),
-            Arguments.of(SiloQuery(SiloAction.aminoAcidInsertions(), True)),
+            SiloQuery(SiloAction.mutations(), True),
+            SiloQuery(SiloAction.aminoAcidMutations(), True),
+            SiloQuery(SiloAction.nucleotideInsertions(), True),
+            SiloQuery(SiloAction.aminoAcidInsertions(), True),
         )
     }
 }