Rewrite transaction handling to avoid potential deadlock

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")

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<String, String>,
+    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)
-            }
-        }
-    }
 }

src/main/kotlin/no/nav/pensjon/opptjening/omsorgsopptjening/start/innlesning/config/AppConfig.kt → 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 ->

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

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)
+    }
+}

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)
+        }
+    }
+}

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<JsonParseException> {
+        assertThrows<UndeclaredThrowableException> {
             barnetrygdService.process()
         }