From 4b667b4a495dea5c72ab0ff247a287c2dcd07d72 Mon Sep 17 00:00:00 2001 From: Jacob Meidell Date: Thu, 14 Sep 2023 12:20:58 +0200 Subject: [PATCH] Rewrite transaction handling to avoid potential deadlock --- build.gradle.kts | 1 + .../domain/BarnetrygdmottakerService.kt | 74 +++++++------------ ...g.kt => DatabaseStartupValidatorConfig.kt} | 18 +---- .../innlesning/config/ObjectMapperConfig.kt | 20 ----- .../start/innlesning/config/RetryConfig.kt | 8 ++ .../config/TransactionManagerConfig.kt | 18 +++++ .../config/TransactionTemplateConfig.kt | 19 +++++ .../domain/BarnetrygdmottakerServiceTest.kt | 5 +- 8 files changed, 79 insertions(+), 84 deletions(-) rename src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/{AppConfig.kt => DatabaseStartupValidatorConfig.kt} (61%) delete mode 100644 src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/ObjectMapperConfig.kt create mode 100644 src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/RetryConfig.kt create mode 100644 src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/TransactionManagerConfig.kt create mode 100644 src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/TransactionTemplateConfig.kt diff --git a/build.gradle.kts b/build.gradle.kts index beea485..56c096c 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -46,6 +46,7 @@ dependencies { implementation("org.springframework.boot:spring-boot-starter-web") implementation("org.springframework.kafka:spring-kafka") implementation("org.springframework.boot:spring-boot-starter-actuator") + implementation("org.springframework:spring-aspects") implementation("no.nav.security:token-validation-spring:$navTokenSupportVersion") implementation("net.logstash.logback:logstash-logback-encoder:$logbackEncoderVersion") diff --git a/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/barnetrygd/domain/BarnetrygdmottakerService.kt b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/barnetrygd/domain/BarnetrygdmottakerService.kt index ff75dca..d15b27d 100644 --- a/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/barnetrygd/domain/BarnetrygdmottakerService.kt +++ b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/barnetrygd/domain/BarnetrygdmottakerService.kt @@ -13,18 +13,16 @@ import no.nav.pensjon.opptjening.omsorgsopptjening.start.innlesning.barnetrygd.e import no.nav.pensjon.opptjening.omsorgsopptjening.start.innlesning.barnetrygd.repository.BarnetrygdmottakerRepository import org.apache.kafka.clients.producer.ProducerRecord import org.slf4j.LoggerFactory -import org.springframework.beans.factory.annotation.Autowired import org.springframework.kafka.core.KafkaTemplate -import org.springframework.stereotype.Component import org.springframework.stereotype.Service -import org.springframework.transaction.annotation.Propagation -import org.springframework.transaction.annotation.Transactional +import org.springframework.transaction.support.TransactionTemplate @Service class BarnetrygdmottakerService( private val client: BarnetrygdClient, private val repo: BarnetrygdmottakerRepository, private val kafkaProducer: KafkaTemplate, + private val transactionTemplate: TransactionTemplate, ) { companion object { private val log = LoggerFactory.getLogger(this::class.java) @@ -34,25 +32,34 @@ class BarnetrygdmottakerService( return client.bestillBarnetrygdmottakere(ar) } - @Transactional(rollbackFor = [Throwable::class]) fun process(): Barnetrygdmottaker? { - return repo.finnNesteUprosesserte()?.let { barnetrygdmottaker -> - Mdc.scopedMdc(barnetrygdmottaker.correlationId) { - Mdc.scopedMdc(barnetrygdmottaker.innlesingId) { - try { - log.info("Prosesserer barnetrygdmottaker med id:${barnetrygdmottaker.id}") + return transactionTemplate.execute { + repo.finnNesteUprosesserte()?.let { barnetrygdmottaker -> + Mdc.scopedMdc(barnetrygdmottaker.correlationId) { + Mdc.scopedMdc(barnetrygdmottaker.innlesingId) { + try { + transactionTemplate.execute { + log.info("Prosesserer barnetrygdmottaker med id:${barnetrygdmottaker.id}") - log.info("Henter detaljer") - client.hentBarnetrygd( - ident = barnetrygdmottaker.ident, - ar = barnetrygdmottaker.år!!, - ).let { - barnetrygdmottaker.handle(it) + log.info("Henter detaljer") + client.hentBarnetrygd( + ident = barnetrygdmottaker.ident, + ar = barnetrygdmottaker.år!!, + ).let { + barnetrygdmottaker.handle(it) + } + } + } catch (ex: Throwable) { + transactionTemplate.execute { + barnetrygdmottaker.retry(ex.toString()).let { + if (it.status is Barnetrygdmottaker.Status.Feilet) { + log.error("Gir opp videre prosessering av melding") + } + repo.updateStatus(it) + } + } + throw ex } - } catch (exception: Throwable) { - log.warn("Exception caught while processing id: ${barnetrygdmottaker.id}, exeption:$exception") - statusoppdatering.markerForRetry(barnetrygdmottaker, exception) - throw exception } } } @@ -114,31 +121,4 @@ class BarnetrygdmottakerService( ) ) } - - @Autowired - private lateinit var statusoppdatering: Statusoppdatering - - /** - * https://docs.spring.io/spring-framework/reference/data-access/transaction/declarative/annotations.html - * - * "In proxy mode (which is the default), only external method calls coming in through the proxy are intercepted. - * This means that self-invocation (in effect, a method within the target object calling another method of the target object) - * does not lead to an actual transaction at runtime even if the invoked method is marked with @Transactional. - * Also, the proxy must be fully initialized to provide the expected behavior, so you should not rely on this feature - * in your initialization code - for example, in a @PostConstruct method." - */ - @Component - private class Statusoppdatering( - private val repo: BarnetrygdmottakerRepository, - ) { - @Transactional(rollbackFor = [Throwable::class], propagation = Propagation.REQUIRES_NEW) - fun markerForRetry(barnetrygdmottaker: Barnetrygdmottaker, exception: Throwable) { - barnetrygdmottaker.retry(exception.toString()).let { - if (it.status is Barnetrygdmottaker.Status.Feilet) { - log.error("Gir opp videre prosessering av melding") - } - repo.updateStatus(it) - } - } - } } \ No newline at end of file diff --git a/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/AppConfig.kt b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/DatabaseStartupValidatorConfig.kt similarity index 61% rename from src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/AppConfig.kt rename to src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/DatabaseStartupValidatorConfig.kt index d1bb994..c9cbaf3 100644 --- a/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/AppConfig.kt +++ b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/DatabaseStartupValidatorConfig.kt @@ -1,32 +1,20 @@ package no.nav.pensjon.opptjening.omsorgsopptjening.start.innlesning.config -import no.nav.security.token.support.spring.api.EnableJwtTokenValidation import org.flywaydb.core.Flyway import org.springframework.beans.factory.config.BeanFactoryPostProcessor import org.springframework.beans.factory.config.ConfigurableListableBeanFactory import org.springframework.context.annotation.Bean -import org.springframework.jdbc.datasource.DataSourceTransactionManager +import org.springframework.context.annotation.Configuration import org.springframework.jdbc.support.DatabaseStartupValidator -import org.springframework.retry.annotation.EnableRetry -import org.springframework.transaction.PlatformTransactionManager -import org.springframework.transaction.annotation.EnableTransactionManagement import javax.sql.DataSource -@EnableJwtTokenValidation -@EnableRetry -@EnableTransactionManagement -class AppConfig { - +@Configuration +class DatabaseStartupValidatorConfig { @Bean fun databaseStartupValidator(dataSource: DataSource) = DatabaseStartupValidator().apply { setDataSource(dataSource) } - @Bean - fun transactionManager(dataSource: DataSource): PlatformTransactionManager { - return DataSourceTransactionManager(dataSource) - } - @Bean fun dependsOnPostProcessor(): BeanFactoryPostProcessor? { return BeanFactoryPostProcessor { bf: ConfigurableListableBeanFactory -> diff --git a/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/ObjectMapperConfig.kt b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/ObjectMapperConfig.kt deleted file mode 100644 index e068a53..0000000 --- a/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/ObjectMapperConfig.kt +++ /dev/null @@ -1,20 +0,0 @@ -package no.nav.pensjon.opptjening.omsorgsopptjening.start.innlesning.config - -import com.fasterxml.jackson.annotation.JsonInclude -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.databind.SerializationFeature -import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule -import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper -import org.springframework.context.annotation.Bean -import org.springframework.context.annotation.Configuration - -@Configuration -class ObjectMapperConfig { - @Bean - fun objectMapper(): ObjectMapper { - return jacksonObjectMapper() - .setSerializationInclusion(JsonInclude.Include.NON_NULL) - .registerModule(JavaTimeModule()) - .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) - } -} \ No newline at end of file diff --git a/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/RetryConfig.kt b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/RetryConfig.kt new file mode 100644 index 0000000..0181df4 --- /dev/null +++ b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/RetryConfig.kt @@ -0,0 +1,8 @@ +package no.nav.pensjon.opptjening.omsorgsopptjening.start.innlesning.config + +import org.springframework.context.annotation.Configuration +import org.springframework.retry.annotation.EnableRetry + +@Configuration +@EnableRetry +class RetryConfig \ No newline at end of file diff --git a/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/TransactionManagerConfig.kt b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/TransactionManagerConfig.kt new file mode 100644 index 0000000..6bafb3c --- /dev/null +++ b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/TransactionManagerConfig.kt @@ -0,0 +1,18 @@ +package no.nav.pensjon.opptjening.omsorgsopptjening.start.innlesning.config + +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration +import org.springframework.jdbc.datasource.DataSourceTransactionManager +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.annotation.EnableTransactionManagement +import javax.sql.DataSource + +@Configuration +@EnableTransactionManagement +class TransactionManagerConfig { + + @Bean + fun transactionManager(dataSource: DataSource): PlatformTransactionManager { + return DataSourceTransactionManager(dataSource) + } +} \ No newline at end of file diff --git a/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/TransactionTemplateConfig.kt b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/TransactionTemplateConfig.kt new file mode 100644 index 0000000..f4ebbc4 --- /dev/null +++ b/src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/TransactionTemplateConfig.kt @@ -0,0 +1,19 @@ +package no.nav.pensjon.opptjening.omsorgsopptjening.start.innlesning.config + +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.TransactionDefinition +import org.springframework.transaction.support.TransactionTemplate + + +@Configuration +class TransactionTemplateConfig { + + @Bean + fun transactionTemplate(transactionManager: PlatformTransactionManager): TransactionTemplate { + return TransactionTemplate(transactionManager).apply { + setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW) + } + } +} diff --git a/src/test/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/barnetrygd/domain/BarnetrygdmottakerServiceTest.kt b/src/test/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/barnetrygd/domain/BarnetrygdmottakerServiceTest.kt index 0e80f72..0b3740d 100644 --- a/src/test/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/barnetrygd/domain/BarnetrygdmottakerServiceTest.kt +++ b/src/test/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/barnetrygd/domain/BarnetrygdmottakerServiceTest.kt @@ -1,6 +1,5 @@ package no.nav.pensjon.opptjening.omsorgsopptjening.start.innlesning.barnetrygd.domain -import com.fasterxml.jackson.core.JsonParseException import com.github.tomakehurst.wiremock.client.WireMock import com.github.tomakehurst.wiremock.core.WireMockConfiguration import com.github.tomakehurst.wiremock.junit5.WireMockExtension @@ -13,6 +12,7 @@ import no.nav.pensjon.opptjening.omsorgsopptjening.start.innlesning.barnetrygd.r import org.apache.kafka.clients.producer.ProducerRecord import org.junit.jupiter.api.Assertions.assertInstanceOf import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows import org.junit.jupiter.api.extension.RegisterExtension import org.mockito.BDDMockito.any import org.mockito.BDDMockito.given @@ -21,6 +21,7 @@ import org.springframework.boot.test.mock.mockito.MockBean import org.springframework.http.HttpHeaders import org.springframework.http.MediaType import org.springframework.kafka.core.KafkaTemplate +import java.lang.reflect.UndeclaredThrowableException import java.time.Clock import java.time.Instant import java.time.temporal.ChronoUnit @@ -207,7 +208,7 @@ class BarnetrygdmottakerServiceTest : SpringContextTest.NoKafka() { barnetrygdmottakerRepository.find(barnetrygdmottaker.id!!)!!.status ) - org.junit.jupiter.api.assertThrows { + assertThrows { barnetrygdService.process() }