From 241542b824c7f26efaa9128d9a1069002eda2072 Mon Sep 17 00:00:00 2001 From: Jan-Niclas Struewer <j.n.struewer@gmail.com> Date: Wed, 7 Aug 2024 18:23:59 +0200 Subject: [PATCH] fix: fixed improper calculation of edge weights when connecting KPI hierarchy and raw values --- .../src/main/resources/application.properties | 2 +- .../iem/kpiCalculator/core/KpiCalculator.kt | 51 +++++---- .../core/hierarchy/KpiCalculationNode.kt | 4 + .../kpiCalculator/core/KpiCalculatorTest.kt | 105 +++++++++++++++--- 4 files changed, 120 insertions(+), 42 deletions(-) diff --git a/app/backend/src/main/resources/application.properties b/app/backend/src/main/resources/application.properties index a062b0ca..f229e22f 100644 --- a/app/backend/src/main/resources/application.properties +++ b/app/backend/src/main/resources/application.properties @@ -54,7 +54,7 @@ spring.jpa.generate-ddl=true spring.jpa.show-sql=false #--- spring.config.activate.on-profile=local -opencode.host=https://gitlab.opencode.de/ +opencode.host=https://gitlab.dev.o4oe.de/ opencode.analyze-private-repos=true # Tool APIs diff --git a/kpi-calculator/core/src/main/kotlin/de/fraunhofer/iem/kpiCalculator/core/KpiCalculator.kt b/kpi-calculator/core/src/main/kotlin/de/fraunhofer/iem/kpiCalculator/core/KpiCalculator.kt index 0b5a07cb..5f2abb90 100644 --- a/kpi-calculator/core/src/main/kotlin/de/fraunhofer/iem/kpiCalculator/core/KpiCalculator.kt +++ b/kpi-calculator/core/src/main/kotlin/de/fraunhofer/iem/kpiCalculator/core/KpiCalculator.kt @@ -8,6 +8,11 @@ import de.fraunhofer.iem.kpiCalculator.model.kpi.hierarchy.KpiHierarchy import de.fraunhofer.iem.kpiCalculator.model.kpi.hierarchy.KpiNode import de.fraunhofer.iem.kpiCalculator.model.kpi.hierarchy.KpiResultHierarchy +private data class TreeUpdate( + val parent: KpiCalculationNode, + val currentNode: KpiCalculationNode, + val rawValueKpis: List<RawValueKpi> +) object KpiCalculator { //XXX: Setup Logger @@ -51,8 +56,7 @@ object KpiCalculator { } val calculationRoot = KpiCalculationNode.from(node) - - val updates: MutableList<Triple<KpiCalculationNode, KpiCalculationNode, List<RawValueKpi>>> = mutableListOf() + val updates: MutableList<TreeUpdate> = mutableListOf() depthFirstTraversal( node = calculationRoot @@ -61,34 +65,33 @@ object KpiCalculator { val parent = currentNode.parent if (!correspondingRawValue.isNullOrEmpty() && parent != null) { - updates.add(Triple(parent, currentNode, correspondingRawValue)) + updates.add(TreeUpdate(parent, currentNode, correspondingRawValue)) } } updates.forEach { - val parent = it.first - val currentNode = it.second - val correspondingRawValue = it.third - - parent.removeChild(currentNode) - - val rawValueNodes = correspondingRawValue.map { rawValue -> - val newNode = KpiCalculationNode( - kind = rawValue.kind, - calculationStrategy = KpiStrategyId.RAW_VALUE_STRATEGY, - parent = parent - ) - newNode.setScore(rawValue.score) - newNode - } - - if (rawValueNodes.isNotEmpty()) { - // TODO: this is currently incorrect and also breaks the remaining nodes weights - val weights = 1.0 / rawValueNodes.size + parent.hierarchyEdges.size - rawValueNodes.forEach { rawValueNode -> - parent.addChild(rawValueNode, weights) + val (parent, currentNode, correspondingRawValue) = it + + parent.getWeight(currentNode)?.let { plannedWeight -> + parent.removeChild(currentNode) + + val rawValueNodes = correspondingRawValue.map { rawValue -> + val newNode = KpiCalculationNode( + kind = rawValue.kind, + calculationStrategy = KpiStrategyId.RAW_VALUE_STRATEGY, + parent = parent + ) + newNode.setScore(rawValue.score) + newNode } + if (rawValueNodes.isNotEmpty()) { + val weights = plannedWeight / rawValueNodes.size + rawValueNodes.forEach { rawValueNode -> + parent.addChild(rawValueNode, weights) + } + + } } } diff --git a/kpi-calculator/core/src/main/kotlin/de/fraunhofer/iem/kpiCalculator/core/hierarchy/KpiCalculationNode.kt b/kpi-calculator/core/src/main/kotlin/de/fraunhofer/iem/kpiCalculator/core/hierarchy/KpiCalculationNode.kt index c5f1989a..a83b3275 100644 --- a/kpi-calculator/core/src/main/kotlin/de/fraunhofer/iem/kpiCalculator/core/hierarchy/KpiCalculationNode.kt +++ b/kpi-calculator/core/src/main/kotlin/de/fraunhofer/iem/kpiCalculator/core/hierarchy/KpiCalculationNode.kt @@ -35,6 +35,10 @@ internal class KpiCalculationNode( _hierarchyEdges.removeIf { it.to == node } } + fun getWeight(node: KpiCalculationNode): Double? { + return hierarchyEdges.find { it.to == node }?.weight + } + fun calculateKpi(): KpiCalculationResult { val strategyData = hierarchyEdges.map { Pair(it.to.score, it.weight) diff --git a/kpi-calculator/core/src/test/kotlin/de/fraunhofer/iem/kpiCalculator/core/KpiCalculatorTest.kt b/kpi-calculator/core/src/test/kotlin/de/fraunhofer/iem/kpiCalculator/core/KpiCalculatorTest.kt index a5f27f67..4b8a4103 100644 --- a/kpi-calculator/core/src/test/kotlin/de/fraunhofer/iem/kpiCalculator/core/KpiCalculatorTest.kt +++ b/kpi-calculator/core/src/test/kotlin/de/fraunhofer/iem/kpiCalculator/core/KpiCalculatorTest.kt @@ -5,6 +5,7 @@ import de.fraunhofer.iem.kpiCalculator.model.kpi.KpiStrategyId import de.fraunhofer.iem.kpiCalculator.model.kpi.RawValueKpi import de.fraunhofer.iem.kpiCalculator.model.kpi.hierarchy.* import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertDoesNotThrow import kotlin.test.assertEquals import kotlin.test.fail @@ -12,20 +13,21 @@ class KpiCalculatorTest { @Test fun calculateDefaultHierarchyKpis() { - val rawValueKpis = listOf( - RawValueKpi(kind = KpiId.VULNERABILITY_SCORE, score = 8), - RawValueKpi(kind = KpiId.VULNERABILITY_SCORE, score = 9), - RawValueKpi(kind = KpiId.CHECKED_IN_BINARIES, score = 100), - RawValueKpi(kind = KpiId.COMMENTS_IN_CODE, score = 80), - RawValueKpi(kind = KpiId.NUMBER_OF_COMMITS, score = 90), - RawValueKpi(kind = KpiId.IS_DEFAULT_BRANCH_PROTECTED, score = 100), - RawValueKpi(kind = KpiId.NUMBER_OF_SIGNED_COMMITS, score = 80), - RawValueKpi(kind = KpiId.SECRETS, score = 80), - RawValueKpi(kind = KpiId.SAST_USAGE, score = 80), - RawValueKpi(kind = KpiId.DOCUMENTATION_INFRASTRUCTURE, score = 80), - ) - val res = KpiCalculator.calculateKpis(DefaultHierarchy.get(), rawValueKpis) - println(res) + assertDoesNotThrow { + val rawValueKpis = listOf( + RawValueKpi(kind = KpiId.VULNERABILITY_SCORE, score = 8), + RawValueKpi(kind = KpiId.VULNERABILITY_SCORE, score = 9), + RawValueKpi(kind = KpiId.CHECKED_IN_BINARIES, score = 100), + RawValueKpi(kind = KpiId.COMMENTS_IN_CODE, score = 80), + RawValueKpi(kind = KpiId.NUMBER_OF_COMMITS, score = 90), + RawValueKpi(kind = KpiId.IS_DEFAULT_BRANCH_PROTECTED, score = 100), + RawValueKpi(kind = KpiId.NUMBER_OF_SIGNED_COMMITS, score = 80), + RawValueKpi(kind = KpiId.SECRETS, score = 80), + RawValueKpi(kind = KpiId.SAST_USAGE, score = 80), + RawValueKpi(kind = KpiId.DOCUMENTATION_INFRASTRUCTURE, score = 80), + ) + KpiCalculator.calculateKpis(DefaultHierarchy.get(), rawValueKpis) + } } @Test @@ -59,7 +61,7 @@ class KpiCalculatorTest { val hierarchy = KpiHierarchy.create(root) val res = KpiCalculator.calculateKpis(hierarchy, rawValueKpis) - val result = res.kpiResult + val result = res.rootNode.kpiResult if (result is KpiCalculationResult.Success) { assertEquals(90, result.score) @@ -107,13 +109,82 @@ class KpiCalculatorTest { val hierarchy = KpiHierarchy.create(root) val res = KpiCalculator.calculateKpis(hierarchy, rawValueKpis) - val result = res.kpiResult + val result = res.rootNode.kpiResult - println(res) if (result is KpiCalculationResult.Incomplete) { assertEquals(90, result.score) } else { fail() } } + + @Test + fun calculateAggregation() { + val rawValueKpis = listOf( + RawValueKpi(kind = KpiId.VULNERABILITY_SCORE, score = 80), + RawValueKpi(kind = KpiId.VULNERABILITY_SCORE, score = 90), + ) + + val root = KpiNode( + kpiId = KpiId.ROOT, strategyType = KpiStrategyId.AGGREGATION_STRATEGY, children = listOf( + KpiEdge( + target = KpiNode( + kpiId = KpiId.VULNERABILITY_SCORE, + strategyType = KpiStrategyId.RAW_VALUE_STRATEGY, + children = listOf() + ), + weight = 1.0 + ), + ) + ) + val hierarchy = KpiHierarchy.create(root) + + val res = KpiCalculator.calculateKpis(hierarchy, rawValueKpis) + val result = res.rootNode.kpiResult + + if (result is KpiCalculationResult.Success) { + assertEquals(85, result.score) + } else { + fail() + } + } + + @Test + fun calculateAggregationKpisIncompleteMixedKinds() { + val rawValueKpis = listOf( + RawValueKpi(kind = KpiId.VULNERABILITY_SCORE, score = 80), + RawValueKpi(kind = KpiId.VULNERABILITY_SCORE, score = 90), + ) + + val root = KpiNode( + kpiId = KpiId.ROOT, strategyType = KpiStrategyId.AGGREGATION_STRATEGY, children = listOf( + KpiEdge( + target = KpiNode( + kpiId = KpiId.VULNERABILITY_SCORE, + strategyType = KpiStrategyId.RAW_VALUE_STRATEGY, + children = listOf() + ), + weight = 0.5 + ), + KpiEdge( + target = KpiNode( + kpiId = KpiId.SAST_USAGE, + strategyType = KpiStrategyId.RAW_VALUE_STRATEGY, + children = listOf() + ), + weight = 0.5 + ) + ) + ) + val hierarchy = KpiHierarchy.create(root) + + val res = KpiCalculator.calculateKpis(hierarchy, rawValueKpis) + val result = res.rootNode.kpiResult + + if (result is KpiCalculationResult.Incomplete) { + assertEquals(85, result.score) + } else { + fail() + } + } } -- GitLab