From 7d877b241d7ed03d4a7a466ea4158911b3e539f1 Mon Sep 17 00:00:00 2001 From: Saku K <6057704+saku-koodari@users.noreply.github.com> Date: Thu, 19 Dec 2024 18:08:07 +0200 Subject: [PATCH] refactor(yki): add successful rows always to database --- .../fi/oph/kitu/csvparsing/CsvExportError.kt | 10 +--- .../fi/oph/kitu/csvparsing/CsvParser.kt | 54 ++++++++++--------- .../main/kotlin/fi/oph/kitu/yki/YkiService.kt | 25 +++++---- 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/server/src/main/kotlin/fi/oph/kitu/csvparsing/CsvExportError.kt b/server/src/main/kotlin/fi/oph/kitu/csvparsing/CsvExportError.kt index faefa0a39..336b630d7 100644 --- a/server/src/main/kotlin/fi/oph/kitu/csvparsing/CsvExportError.kt +++ b/server/src/main/kotlin/fi/oph/kitu/csvparsing/CsvExportError.kt @@ -32,15 +32,9 @@ abstract class CsvExportError( } } -class CsvExportException( - val errors: Iterable, - message: String? = "Unable to convert string to csv, because the string had ${errors.count()} error(s).", - cause: Throwable? = null, -) : Throwable(message, cause) - -fun LoggingEventBuilder.addErrors(exception: CsvExportException) { +fun LoggingEventBuilder.addErrors(errors: Iterable) { // add all errors to log - exception.errors.forEachIndexed { i, error -> + errors.forEachIndexed { i, error -> this.add("serialization.error[$i].index" to i) for (kvp in error.keyValues) { this.add("serialization.error[$i].${kvp.first}" to kvp.second) diff --git a/server/src/main/kotlin/fi/oph/kitu/csvparsing/CsvParser.kt b/server/src/main/kotlin/fi/oph/kitu/csvparsing/CsvParser.kt index c6f7b3138..db1ddba9f 100644 --- a/server/src/main/kotlin/fi/oph/kitu/csvparsing/CsvParser.kt +++ b/server/src/main/kotlin/fi/oph/kitu/csvparsing/CsvParser.kt @@ -1,6 +1,5 @@ package fi.oph.kitu.csvparsing -import com.fasterxml.jackson.databind.MappingIterator import com.fasterxml.jackson.databind.exc.InvalidFormatException import com.fasterxml.jackson.databind.module.SimpleModule import com.fasterxml.jackson.dataformat.csv.CsvMapper @@ -10,6 +9,7 @@ import fi.oph.kitu.logging.add import org.ietf.jgss.Oid import org.slf4j.spi.LoggingEventBuilder import java.io.ByteArrayOutputStream +import java.lang.RuntimeException import kotlin.reflect.full.findAnnotation class CsvParser( @@ -92,7 +92,7 @@ class CsvParser( /** * Converts retrieved String response into a list that is the type of Body. */ - inline fun convertCsvToData(csvString: String): List { + inline fun convertCsvtToResults(csvString: String): Iterable> { if (csvString.isBlank()) { event.add("serialization.isEmptyList" to true) return emptyList() @@ -103,41 +103,47 @@ class CsvParser( val csvMapper = getCsvMapper() val schema = getSchema(csvMapper) - // the lines are needed to read line by line in order to distinguish all erroneous lines - val errors = mutableListOf() - val iterator = csvMapper .readerFor(T::class.java) .with(schema) .readValues(csvString) - val data = - iterator.toDataWithErrorHandling { index, e -> - when (e) { - is InvalidFormatException -> errors.add(InvalidFormatCsvExportError(index, e)) - else -> errors.add(SimpleCsvExportError(index, e)) - } - } - - if (errors.isEmpty()) { - return data + // the lines are needed to read line by line in order to distinguish all erroneous lines + val data = mutableListOf>() + while (iterator.hasNext()) { + data.add(runCatching { iterator.nextValue() }) } - throw CsvExportException(errors) + return data } } -fun MappingIterator.toDataWithErrorHandling( - onFailure: (index: Int, exception: Throwable) -> Unit = { _, _ -> }, +fun Iterable>.foldWithErrors( + continueOnError: Boolean, + action: (Iterable) -> Unit, ): List { + val errors = mutableListOf() val data = mutableListOf() - var index = 0 - while (this.hasNext()) { - runCatching { this.nextValue() } - .onSuccess { d -> data.add(d) } - .onFailure { e -> onFailure(index, e) } - .also { index++ } + // Add errors to error list + this.forEachIndexed { index, result -> + result + .onFailure { e -> + when (e) { + is InvalidFormatException -> errors.add(InvalidFormatCsvExportError(index, e)) + else -> errors.add(SimpleCsvExportError(index, e)) + } + }.onSuccess { row -> data.add(row) } + + if (errors.isNotEmpty()) { + action(errors) + + if (!continueOnError) { + throw RuntimeException( + "Unable to convert string to csv, because the string had ${errors.count()} error(s).", + ) + } + } } return data diff --git a/server/src/main/kotlin/fi/oph/kitu/yki/YkiService.kt b/server/src/main/kotlin/fi/oph/kitu/yki/YkiService.kt index 5ca37b493..dfeacb588 100644 --- a/server/src/main/kotlin/fi/oph/kitu/yki/YkiService.kt +++ b/server/src/main/kotlin/fi/oph/kitu/yki/YkiService.kt @@ -1,9 +1,9 @@ package fi.oph.kitu.yki import fi.oph.kitu.PeerService -import fi.oph.kitu.csvparsing.CsvExportException import fi.oph.kitu.csvparsing.CsvParser import fi.oph.kitu.csvparsing.addErrors +import fi.oph.kitu.csvparsing.foldWithErrors import fi.oph.kitu.logging.add import fi.oph.kitu.logging.addHttpResponse import fi.oph.kitu.logging.withEvent @@ -58,17 +58,11 @@ class YkiService( event.addHttpResponse(PeerService.Solki, "suoritukset", response) - val suoritukset = - try { - parser.convertCsvToData(response.body ?: "") - } catch (e: CsvExportException) { - event.addErrors(e) - - if (!importSuorituksetContinueOnError) { - throw e - } else { - listOf() - } + val suoritukset = mutableListOf() + parser + .convertCsvtToResults(response.body ?: "") + .foldWithErrors(importSuorituksetContinueOnError) { + event.addErrors(it) } if (dryRun != true) { @@ -91,7 +85,12 @@ class YkiService( event.addHttpResponse(PeerService.Solki, "arvioijat", response) val arvioijat = - parser.convertCsvToData(response.body ?: throw Error.EmptyArvioijatResponse()) + parser + .convertCsvtToResults( + response.body ?: throw Error.EmptyArvioijatResponse(), + ).foldWithErrors(false) { + event.addErrors(it) + } event.add("yki.arvioijat.receivedCount" to arvioijat.size) if (arvioijat.isEmpty()) {