pr-fix(yki): improve logging

this includes removing dry-run from headers and simplifying code
Opetushallitus · Oct 17, 2024 · de74059 · de74059
1 parent fac2835
commit de74059
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 33 deletions.
diff --git a/server/src/main/kotlin/fi/oph/kitu/dev/MockYkiControllerApi.kt b/server/src/main/kotlin/fi/oph/kitu/dev/MockYkiControllerApi.kt
@@ -8,11 +8,11 @@ import org.slf4j.LoggerFactory
 import org.springframework.boot.SpringApplication
 import org.springframework.context.annotation.Profile
 import org.springframework.core.env.Environment
-import org.springframework.http.HttpHeaders
 import org.springframework.http.HttpStatus
 import org.springframework.http.ResponseEntity
 import org.springframework.web.bind.annotation.RestController
 import org.springframework.web.context.WebApplicationContext
+import java.math.BigDecimal
 import java.time.LocalDate
 import kotlin.system.exitProcess
 
@@ -34,23 +34,20 @@ class MockYkiControllerApi(
     }
 
     override fun getArvioijat() =
-        ResponseEntity
-            .ok()
-            .headers(HttpHeaders().apply { set("dry-run", "true") })
-            .body(
-                """"09876","010101A961P","Arvioija","Arttu","[email protected]","Testiosoite 7357","00100","HELSINKI",0,"rus","PT+KT" """,
-            )
+        ResponseEntity(
+            """"09876","010101A961P","Arvioija","Arttu","[email protected]","Testiosoite 7357","00100","HELSINKI",0,"rus","PT+KT" """,
+            HttpStatus.OK,
+        )
 
     override fun getSuoritukset(arvioitu: LocalDate?): ResponseEntity<String> =
-        ResponseEntity
-            .ok()
-            .headers(HttpHeaders().apply { set("dry-run", "true") })
-            .body(
-                """"1.2.246.562.24.99999999999","Suorittaja","Sulevi",2022-11-12,"fin","KT","1.2.246.562.10.373218511910","Iisalmen kansalaisopisto",2,2,1,3,2,2""",
-            )
+        ResponseEntity(
+            """"1.2.246.562.24.99999999999","Suorittaja","Sulevi",2022-11-12,"fin","KT","1.2.246.562.10.373218511910","Iisalmen kansalaisopisto",2,2,1,3,2,2""",
+            HttpStatus.OK,
+        )
 
     override fun triggerYkiImportSuoritukset(
         dryRun: Boolean?,
         lastSeen: LocalDate?,
-    ): ResponseEntity<Unit> = ResponseEntity(ykiService.importYkiSuoritukset(lastSeen, dryRun), HttpStatus.OK)
+    ): ResponseEntity<BigDecimal> =
+        ResponseEntity(ykiService.importYkiSuoritukset(lastSeen, dryRun).toBigDecimal(), HttpStatus.OK)
 }
diff --git a/server/src/main/kotlin/fi/oph/kitu/generated/api/YkiControllerApi.kt b/server/src/main/kotlin/fi/oph/kitu/generated/api/YkiControllerApi.kt
@@ -34,6 +34,7 @@ interface YkiControllerApi {
     @RequestMapping(
         method = [RequestMethod.GET],
         value = ["/dev/yki/trigger-schedule/yki-import-suoritukset"],
+        produces = ["plain/text"],
     )
     fun triggerYkiImportSuoritukset(
         @RequestHeader(value = "dry-run", required = false) dryRun: kotlin.Boolean?,
@@ -43,5 +44,5 @@ interface YkiControllerApi {
         ) @org.springframework.format.annotation.DateTimeFormat(
             iso = org.springframework.format.annotation.DateTimeFormat.ISO.DATE,
         ) lastSeen: java.time.LocalDate?,
-    ): ResponseEntity<Unit>
+    ): ResponseEntity<java.math.BigDecimal>
 }
diff --git a/server/src/main/kotlin/fi/oph/kitu/yki/YkiRepository.kt b/server/src/main/kotlin/fi/oph/kitu/yki/YkiRepository.kt
@@ -4,7 +4,7 @@ import org.springframework.stereotype.Repository
 
 @Repository
 class YkiRepository {
-    fun insertSuoritukset(suoritukset: List<YkiSuoritus>): Void = throw NotImplementedError()
+    fun insertSuoritukset(suoritukset: List<YkiSuoritus>): List<YkiSuoritus> = throw NotImplementedError()
 
-    fun insertSuoritus(suoritus: YkiSuoritus): Void = throw NotImplementedError()
+    fun insertSuoritus(suoritus: YkiSuoritus): YkiSuoritus = throw NotImplementedError()
 }
diff --git a/server/src/main/kotlin/fi/oph/kitu/yki/YkiScheduledTasks.kt b/server/src/main/kotlin/fi/oph/kitu/yki/YkiScheduledTasks.kt
@@ -3,20 +3,46 @@ package fi.oph.kitu.yki
 import com.github.kagkarlsson.scheduler.task.Task
 import com.github.kagkarlsson.scheduler.task.helper.Tasks
 import com.github.kagkarlsson.scheduler.task.schedule.Schedules
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
 import org.springframework.beans.factory.annotation.Value
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
 import org.springframework.context.annotation.Bean
 import org.springframework.context.annotation.Configuration
+import kotlin.math.log
 
 @Configuration
 @ConditionalOnProperty(name = ["kitu.yki.scheduling.enabled"], matchIfMissing = false)
 class YkiScheduledTasks {
     @Value("\${kitu.yki.scheduling.import.schedule}")
     lateinit var ykiImportSchedule: String
 
+    private val logger: Logger = LoggerFactory.getLogger(javaClass)
+
+    private val dryRun: Boolean? = false
+
     @Bean
     fun dailyImport(ykiService: YkiService): Task<Void> =
         Tasks
             .recurring("YKI-import", Schedules.parseSchedule(ykiImportSchedule))
-            .execute { _, _ -> ykiService.importYkiSuoritukset() }
+            .execute { _, _ -> wrapLogging { ykiService.importYkiSuoritukset() } }
+
+    fun wrapLogging(method: () -> Int) {
+        logger.atTrace().log("imports started")
+        try {
+            val res = method()
+            logger
+                .atInfo()
+                .addKeyValue("dryRun", dryRun)
+                .addKeyValue("importSize", res)
+                .log("imports done")
+        } catch (e: Exception) {
+            logger
+                .atError()
+                .addKeyValue("dryRun", dryRun)
+                .setCause(e)
+                .log("imports failed")
+            throw e
+        }
+    }
 }
diff --git a/server/src/main/kotlin/fi/oph/kitu/yki/YkiService.kt b/server/src/main/kotlin/fi/oph/kitu/yki/YkiService.kt
@@ -1,7 +1,6 @@
 package fi.oph.kitu.yki
 
 import fi.oph.kitu.asCsv
-import fi.oph.kitu.isDryRun
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 import org.springframework.beans.factory.annotation.Qualifier
@@ -21,30 +20,24 @@ class YkiService(
     fun importYkiSuoritukset(
         lastSeen: LocalDate? = null,
         dryRun: Boolean? = null,
-    ) {
-        var isDryRun = false
+    ): Int {
         val suoritukset =
             ykiRestClient
                 .get()
                 .uri("suoritukset")
-                .exchange { _, res ->
-                    isDryRun = (dryRun == true) || res.isDryRun()
-                    val bodyAsString =
-                        res.bodyTo(String::class.java) ?: throw RestClientException("Response body is null")
-                    bodyAsString.asCsv<YkiSuoritus>()
-                }
+                .retrieve()
+                .body(String::class.java)
+                ?.asCsv<YkiSuoritus>() ?: throw RestClientException("Response body is empty")
 
         if (suoritukset.isEmpty()) {
-            logger.error("YKI responded with empty data.")
             throw RestClientException("The response is empty")
         }
 
-        if (isDryRun) {
-            logger.info("dry run complete")
-            return
+        if (dryRun == true) {
+            return 0
         }
 
-        repository.insertSuoritukset(suoritukset)
-        logger.info("suoritukset was added.")
+        val res = repository.insertSuoritukset(suoritukset)
+        return res.size
     }
 }
diff --git a/server/src/main/resources/static/open-api.yaml b/server/src/main/resources/static/open-api.yaml
@@ -154,6 +154,11 @@ paths:
       responses:
         "200":
           description: "Triggered"
+          content:
+            "plain/text":
+              schema:
+                type: number
+                description: "Number of suoritukset that was imported"
         "500":
           description: "Failed"