diff --git a/lapis2/build.gradle b/lapis2/build.gradle index 87940d48..73b4d742 100644 --- a/lapis2/build.gradle +++ b/lapis2/build.gradle @@ -29,6 +29,8 @@ repositories { dependencies { implementation 'org.springframework.boot:spring-boot-starter-web' + implementation 'org.springframework.boot:spring-boot-starter-actuator' + implementation 'org.springframework.boot:spring-boot-starter-cache' implementation 'com.fasterxml.jackson.module:jackson-module-kotlin' implementation 'org.jetbrains.kotlin:kotlin-reflect' implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml' @@ -39,6 +41,7 @@ dependencies { implementation 'org.antlr:antlr4-runtime:4.13.1' implementation 'org.apache.commons:commons-csv:1.10.0' implementation 'com.github.luben:zstd-jni:1.5.5-11' + implementation 'com.github.ben-manes.caffeine:caffeine:3.1.8' testImplementation('org.springframework.boot:spring-boot-starter-test') { exclude group: "org.mockito" diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/LapisSpringConfig.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/LapisSpringConfig.kt index 3ef803b8..0b928ba1 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/LapisSpringConfig.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/LapisSpringConfig.kt @@ -20,12 +20,16 @@ import org.genspectrum.lapis.openApi.buildOpenApiSchema import org.genspectrum.lapis.util.TimeFactory import org.genspectrum.lapis.util.YamlObjectMapper import org.springframework.beans.factory.annotation.Value +import org.springframework.cache.annotation.EnableCaching import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration +import org.springframework.scheduling.annotation.EnableScheduling import org.springframework.web.filter.CommonsRequestLoggingFilter import java.io.File @Configuration +@EnableScheduling +@EnableCaching class LapisSpringConfig { @Bean fun openAPI( diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt index ef291cbe..8ddfd55b 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt @@ -111,6 +111,7 @@ private class ProtectedDataAuthorizationFilter( private val WHITELISTED_PATH_PREFIXES = listOf( "/swagger-ui", "/api-docs", + "/actuator", "/sample$DATABASE_CONFIG_ROUTE", "/sample$REFERENCE_GENOME_ROUTE", ) diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/scheduler/DataVersionCacheInvalidator.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/scheduler/DataVersionCacheInvalidator.kt new file mode 100644 index 00000000..25daec60 --- /dev/null +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/scheduler/DataVersionCacheInvalidator.kt @@ -0,0 +1,43 @@ +package org.genspectrum.lapis.scheduler + +import mu.KotlinLogging +import org.genspectrum.lapis.silo.CachedSiloClient +import org.genspectrum.lapis.silo.SILO_QUERY_CACHE_NAME +import org.springframework.cache.annotation.CacheEvict +import org.springframework.scheduling.annotation.Scheduled +import org.springframework.stereotype.Component +import java.util.concurrent.TimeUnit + +private val log = KotlinLogging.logger {} + +@Component +class DataVersionCacheInvalidator( + private val cachedSiloClient: CachedSiloClient, + private val cacheClearer: CacheClearer, +) { + private var currentlyCachedDataVersion = "uninitialized" + + @Scheduled(fixedRate = 1, timeUnit = TimeUnit.SECONDS) + @Synchronized + fun invalidateSiloCache() { + log.debug { "checking for data version change" } + + val info = cachedSiloClient.callInfo() + if (info.dataVersion != currentlyCachedDataVersion) { + log.info { + "Invalidating cache, old data version: $currentlyCachedDataVersion, " + + "new data version: ${info.dataVersion}" + } + cacheClearer.clearCache() + currentlyCachedDataVersion = info.dataVersion + } + } +} + +@Component +class CacheClearer { + @CacheEvict(SILO_QUERY_CACHE_NAME, allEntries = true) + fun clearCache() { + log.info { "Clearing cache $SILO_QUERY_CACHE_NAME" } + } +} diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloClient.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloClient.kt index b9f5af34..06cb72ce 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloClient.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloClient.kt @@ -7,10 +7,12 @@ import org.genspectrum.lapis.controller.LapisHeaders.REQUEST_ID import org.genspectrum.lapis.logging.RequestIdContext import org.genspectrum.lapis.response.InfoData import org.springframework.beans.factory.annotation.Value +import org.springframework.cache.annotation.Cacheable import org.springframework.http.HttpHeaders import org.springframework.http.HttpStatus import org.springframework.http.MediaType import org.springframework.stereotype.Component +import org.springframework.web.context.request.RequestContextHolder import java.net.URI import java.net.http.HttpClient import java.net.http.HttpRequest @@ -23,14 +25,34 @@ const val SILO_RESPONSE_MAX_LOG_LENGTH = 10_000 @Component class SiloClient( + private val cachedSiloClient: CachedSiloClient, + private val dataVersion: DataVersion, +) { + fun sendQuery(query: SiloQuery): 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 + } +} + +const val SILO_QUERY_CACHE_NAME = "siloQueryCache" + +@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() - fun sendQuery(query: SiloQuery): ResponseType { + @Cacheable(SILO_QUERY_CACHE_NAME, condition = "#query.action.cacheable") + fun sendQuery(query: SiloQuery): WithDataVersion { val queryJson = objectMapper.writeValueAsString(query) log.info { "Calling SILO: $queryJson" } @@ -41,7 +63,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) @@ -63,7 +88,7 @@ class SiloClient( val request = HttpRequest.newBuilder(uri) .apply(buildRequest) .apply { - if (requestIdContext.requestId != null) { + if (RequestContextHolder.getRequestAttributes() != null && requestIdContext.requestId != null) { header(REQUEST_ID, requestIdContext.requestId) } } @@ -112,7 +137,6 @@ class SiloClient( ) } - addDataVersionToRequestScope(response) return response } @@ -124,10 +148,6 @@ class SiloClient( null } - private fun addDataVersionToRequestScope(response: HttpResponse) { - dataVersion.dataVersion = getDataVersion(response) - } - private fun getDataVersion(response: HttpResponse): String { return response.headers().firstValue("data-version").orElse("") } @@ -141,4 +161,9 @@ data class SiloQueryResponse( val queryResult: ResponseType, ) +data class WithDataVersion( + val dataVersion: String, + val queryResult: ResponseType, +) + data class SiloErrorResponse(val error: String, val message: String) diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt index f48ab3b5..d98de20a 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt @@ -34,6 +34,7 @@ interface CommonActionFields { sealed class SiloAction( @JsonIgnore val typeReference: TypeReference>, + @JsonIgnore val cacheable: Boolean, ) : CommonActionFields { companion object { fun aggregated( @@ -104,7 +105,7 @@ sealed class SiloAction( override val limit: Int? = null, override val offset: Int? = null, val type: String = "Aggregated", - ) : SiloAction>(AggregationDataTypeReference()) + ) : SiloAction>(AggregationDataTypeReference(), cacheable = false) @JsonInclude(JsonInclude.Include.NON_EMPTY) private data class MutationsAction( @@ -113,7 +114,7 @@ sealed class SiloAction( override val limit: Int? = null, override val offset: Int? = null, val type: String = "Mutations", - ) : SiloAction>(MutationDataTypeReference()) + ) : SiloAction>(MutationDataTypeReference(), cacheable = true) @JsonInclude(JsonInclude.Include.NON_EMPTY) private data class AminoAcidMutationsAction( @@ -122,7 +123,7 @@ sealed class SiloAction( override val limit: Int? = null, override val offset: Int? = null, val type: String = "AminoAcidMutations", - ) : SiloAction>(AminoAcidMutationDataTypeReference()) + ) : SiloAction>(AminoAcidMutationDataTypeReference(), cacheable = true) @JsonInclude(JsonInclude.Include.NON_EMPTY) private data class DetailsAction( @@ -131,7 +132,7 @@ sealed class SiloAction( override val limit: Int? = null, override val offset: Int? = null, val type: String = "Details", - ) : SiloAction>(DetailsDataTypeReference()) + ) : SiloAction>(DetailsDataTypeReference(), cacheable = false) @JsonInclude(JsonInclude.Include.NON_EMPTY) private data class NucleotideInsertionsAction( @@ -139,7 +140,7 @@ sealed class SiloAction( override val limit: Int? = null, override val offset: Int? = null, val type: String = "Insertions", - ) : SiloAction>(InsertionDataTypeReference()) + ) : SiloAction>(InsertionDataTypeReference(), cacheable = true) @JsonInclude(JsonInclude.Include.NON_EMPTY) private data class AminoAcidInsertionsAction( @@ -147,7 +148,7 @@ sealed class SiloAction( override val limit: Int? = null, override val offset: Int? = null, val type: String = "AminoAcidInsertions", - ) : SiloAction>(InsertionDataTypeReference()) + ) : SiloAction>(InsertionDataTypeReference(), cacheable = true) @JsonInclude(JsonInclude.Include.NON_EMPTY) private data class SequenceAction( @@ -156,7 +157,7 @@ sealed class SiloAction( override val offset: Int? = null, val type: SequenceType, val sequenceName: String, - ) : SiloAction>(SequenceDataTypeReference()) + ) : SiloAction>(SequenceDataTypeReference(), cacheable = false) } sealed class SiloFilterExpression(val type: String) diff --git a/lapis2/src/main/resources/application.properties b/lapis2/src/main/resources/application.properties index cc20983f..701b36a3 100644 --- a/lapis2/src/main/resources/application.properties +++ b/lapis2/src/main/resources/application.properties @@ -2,3 +2,12 @@ springdoc.default-produces-media-type=application/json springdoc.api-docs.path=/api-docs springdoc.swagger-ui.operationsSorter=alpha server.forward-headers-strategy=framework + +spring.cache.cache-names=siloQueryCache +spring.cache.caffeine.spec=maximumSize=50000 + +management.endpoints.enabled-by-default=false +management.endpoint.health.enabled=true +management.endpoint.caches.enabled=true +management.endpoint.metrics.enabled=true +management.endpoints.web.exposure.include=health,caches,metrics diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloClientTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloClientTest.kt index 995aaf3f..ef87009b 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloClientTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloClientTest.kt @@ -8,6 +8,7 @@ import org.genspectrum.lapis.response.AggregationData import org.genspectrum.lapis.response.DetailsData import org.genspectrum.lapis.response.MutationData import org.genspectrum.lapis.response.SequenceData +import org.genspectrum.lapis.scheduler.DataVersionCacheInvalidator import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.containsInAnyOrder import org.hamcrest.Matchers.containsString @@ -19,8 +20,11 @@ import org.junit.jupiter.api.AfterEach 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.MethodSource import org.mockserver.client.MockServerClient import org.mockserver.integration.ClientAndServer +import org.mockserver.matchers.Times import org.mockserver.model.HttpRequest.request import org.mockserver.model.HttpResponse import org.mockserver.model.HttpResponse.response @@ -32,19 +36,30 @@ private const val MOCK_SERVER_PORT = 1080 private const val REQUEST_ID_VALUE = "someRequestId" +private const val DATA_VERSION_HEADER = "data-version" + @SpringBootTest(properties = ["silo.url=http://localhost:$MOCK_SERVER_PORT"]) class SiloClientTest( @Autowired private val underTest: SiloClient, @Autowired private val requestIdContext: RequestIdContext, + @Autowired private val dataVersion: DataVersion, ) { private lateinit var mockServer: ClientAndServer - private val someQuery = SiloQuery(SiloAction.aggregated(), StringEquals("theColumn", "theValue")) + private lateinit var someQuery: SiloQuery<*> + + private var counter = 0 @BeforeEach - fun setupMockServer() { + fun setup() { mockServer = ClientAndServer.startClientAndServer(MOCK_SERVER_PORT) requestIdContext.requestId = REQUEST_ID_VALUE + + someQuery = SiloQuery( + SiloAction.aggregated(), + StringEquals("theColumn", "a value that is difference for each test method: $counter"), + ) + counter++ } @AfterEach @@ -335,15 +350,186 @@ class SiloClientTest( assertThat(exception.retryAfter, `is`(nullValue())) } - private fun expectQueryRequestAndRespondWith(httpResponse: HttpResponse?) { - MockServerClient("localhost", MOCK_SERVER_PORT) - .`when`( - request() - .withMethod("POST") - .withPath("/query") - .withContentType(MediaType.APPLICATION_JSON) - .withHeader("X-Request-Id", REQUEST_ID_VALUE), - ) - .respond(httpResponse) + @ParameterizedTest + @MethodSource("getQueriesThatShouldNotBeCached") + fun `GIVEN an action that should not be cached WHEN I send the same request twice THEN server is called twice`( + query: SiloQuery<*>, + ) { + val errorMessage = "make this fail so that we see a difference on the second call" + + expectQueryRequestAndRespondWith( + response() + .withStatusCode(200) + .withBody("""{"queryResult": []}"""), + Times.exactly(1), + ) + expectQueryRequestAndRespondWith( + response() + .withStatusCode(500) + .withBody(errorMessage), + Times.exactly(1), + ) + + underTest.sendQuery(query) + + val exception = assertThrows { underTest.sendQuery(query) } + assertThat(exception.message, containsString(errorMessage)) + } + + @ParameterizedTest + @MethodSource("getQueriesThatShouldBeCached") + fun `GIVEN an action that should be cached WHEN I send the same request twice THEN second time is cached`( + query: SiloQuery<*>, + ) { + expectQueryRequestAndRespondWith( + response() + .withStatusCode(200) + .withBody("""{"queryResult": []}"""), + Times.once(), + ) + + val result1 = underTest.sendQuery(query) + val result2 = underTest.sendQuery(query) + + assertThat(result1, `is`(result2)) + } + + @Test + fun `GIVEN an action that should be cached WHEN I send request twice THEN data version is populated`() { + val dataVersionValue = "someDataVersion" + expectInfoCallAndReturnDataVersion(dataVersionValue) + + expectQueryRequestAndRespondWith( + response() + .withStatusCode(200) + .withHeader(DATA_VERSION_HEADER, 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)) + } + + companion object { + @JvmStatic + val queriesThatShouldNotBeCached = listOf( + 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( + SiloQuery(SiloAction.mutations(), True), + SiloQuery(SiloAction.aminoAcidMutations(), True), + SiloQuery(SiloAction.nucleotideInsertions(), True), + SiloQuery(SiloAction.aminoAcidInsertions(), True), + ) + } +} + +@SpringBootTest(properties = ["silo.url=http://localhost:$MOCK_SERVER_PORT"]) +class SiloClientAndCacheInvalidatorTest( + @Autowired private val siloClient: SiloClient, + @Autowired private val dataVersionCacheInvalidator: DataVersionCacheInvalidator, + @Autowired private val requestIdContext: RequestIdContext, + @Autowired private val dataVersion: DataVersion, +) { + private lateinit var mockServer: ClientAndServer + + val someQuery = SiloQuery(SiloAction.mutations(), True) + val anotherQuery = SiloQuery(SiloAction.aminoAcidMutations(), True) + val firstDataVersion = "1" + val secondDataVersion = "2" + + @BeforeEach + fun setup() { + mockServer = ClientAndServer.startClientAndServer(MOCK_SERVER_PORT) + requestIdContext.requestId = REQUEST_ID_VALUE + } + + @AfterEach + fun stopServer() { + mockServer.stop() + } + + @Test + fun `GIVEN there is a new data version WHEN the cache invalidator checks THEN the cache should be cleared`() { + expectInfoCallAndReturnDataVersion(firstDataVersion, Times.once()) + dataVersionCacheInvalidator.invalidateSiloCache() + + assertThatResultIsCachedOnSecondRequest() + + expectInfoCallAndReturnDataVersion(secondDataVersion, Times.once()) + dataVersionCacheInvalidator.invalidateSiloCache() + + assertThatCacheIsNotHit() } + + private fun assertThatResultIsCachedOnSecondRequest() { + expectQueryRequestAndRespondWith( + response() + .withStatusCode(200) + .withHeader(DATA_VERSION_HEADER, firstDataVersion) + .withBody("""{"queryResult": []}"""), + Times.once(), + ) + + siloClient.sendQuery(someQuery) + siloClient.sendQuery(someQuery) + assertThat(dataVersion.dataVersion, `is`(firstDataVersion)) + } + + private fun assertThatCacheIsNotHit() { + val errorMessage = "This error should appear" + expectQueryRequestAndRespondWith( + response() + .withStatusCode(500) + .withHeader(DATA_VERSION_HEADER, secondDataVersion) + .withBody(errorMessage), + Times.once(), + ) + + val exception = assertThrows { siloClient.sendQuery(someQuery) } + assertThat(exception.message, containsString(errorMessage)) + } +} + +private fun expectQueryRequestAndRespondWith( + httpResponse: HttpResponse, + times: Times = Times.unlimited(), +) { + MockServerClient("localhost", MOCK_SERVER_PORT) + .`when`( + request() + .withMethod("POST") + .withPath("/query") + .withContentType(MediaType.APPLICATION_JSON) + .withHeader("X-Request-Id", REQUEST_ID_VALUE), + times, + ) + .respond(httpResponse) +} + +private fun expectInfoCallAndReturnDataVersion( + dataVersion: String, + times: Times = Times.unlimited(), +) { + MockServerClient("localhost", MOCK_SERVER_PORT) + .`when`( + request() + .withMethod("GET") + .withPath("/info"), + times, + ) + .respond(response().withStatusCode(200).withHeader(DATA_VERSION_HEADER, dataVersion)) } diff --git a/lapis2/src/test/resources/logback-test.xml b/lapis2/src/test/resources/logback-test.xml index 1ddb2cc7..484b015c 100644 --- a/lapis2/src/test/resources/logback-test.xml +++ b/lapis2/src/test/resources/logback-test.xml @@ -9,6 +9,10 @@ + + + +