Skip to content

Commit baedafb

Browse files
authored
fix: remove scenario addLastRunInfo on parent and root (#689)
1 parent 3285318 commit baedafb

File tree

3 files changed

+5
-254
lines changed

3 files changed

+5
-254
lines changed

scenario/src/main/kotlin/com/cosmotech/scenario/service/ScenarioExtensions.kt

Lines changed: 0 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -2,86 +2,12 @@
22
// Licensed under the MIT license.
33
package com.cosmotech.scenario.service
44

5-
import com.cosmotech.api.exceptions.CsmResourceNotFoundException
65
import com.cosmotech.api.rbac.ROLE_NONE
76
import com.cosmotech.api.rbac.model.RbacAccessControl
87
import com.cosmotech.api.rbac.model.RbacSecurity
98
import com.cosmotech.scenario.domain.Scenario
109
import com.cosmotech.scenario.domain.ScenarioAccessControl
1110
import com.cosmotech.scenario.domain.ScenarioSecurity
12-
import org.slf4j.LoggerFactory
13-
14-
// PROD-8051 : add parent and master last runs data
15-
internal fun Scenario.addLastRunsInfo(
16-
scenarioServiceImpl: ScenarioServiceImpl,
17-
organizationId: String,
18-
workspaceId: String
19-
): Scenario {
20-
val scenarioWithLastRunsInfo =
21-
listOf(this).addLastRunsInfo(scenarioServiceImpl, organizationId, workspaceId).first()
22-
this.parentLastRun = scenarioWithLastRunsInfo.parentLastRun
23-
this.rootLastRun = scenarioWithLastRunsInfo.rootLastRun
24-
return this
25-
}
26-
27-
// Grouping for performance reasons. This allows to issue one call per parentId
28-
// and one call per rootId
29-
internal fun List<Scenario>.addLastRunsInfo(
30-
scenarioServiceImpl: ScenarioServiceImpl,
31-
organizationId: String,
32-
workspaceId: String
33-
): List<Scenario> {
34-
val logger =
35-
LoggerFactory.getLogger("com.cosmotech.scenario.azure.ScenarioExtensions#addLastRunsInfo")
36-
return this.groupBy { it.parentId }
37-
.flatMap { (parentId, scenarios) ->
38-
if (!parentId.isNullOrBlank()) {
39-
val parentLastRun =
40-
try {
41-
scenarioServiceImpl
42-
.findScenarioByIdNoState(organizationId, workspaceId, parentId)
43-
.lastRun
44-
} catch (iae: CsmResourceNotFoundException) {
45-
// There might be cases where the parent no longer exists
46-
val messageFormat =
47-
"Parent scenario #{} not found " +
48-
"=> returning null as 'parentLastRun' " +
49-
"for all its {} direct child(ren)"
50-
logger.debug(messageFormat, parentId, scenarios.size)
51-
if (logger.isTraceEnabled) {
52-
logger.trace(messageFormat, parentId, scenarios.size, iae)
53-
}
54-
null
55-
}
56-
scenarios.forEach { it.parentLastRun = parentLastRun }
57-
}
58-
scenarios
59-
}
60-
.groupBy { it.rootId }
61-
.flatMap { (rootId, scenarios) ->
62-
if (!rootId.isNullOrBlank()) {
63-
val rootLastRun =
64-
try {
65-
scenarioServiceImpl
66-
.findScenarioByIdNoState(organizationId, workspaceId, rootId)
67-
.lastRun
68-
} catch (iae: CsmResourceNotFoundException) {
69-
// There might be cases where the root scenario no longer exists
70-
val messageFormat =
71-
"Root scenario #{} not found" +
72-
"=> returning null as 'rootLastRun' " +
73-
"for all its {} child(ren)"
74-
logger.debug(messageFormat, rootId, scenarios.size)
75-
if (logger.isTraceEnabled) {
76-
logger.trace(messageFormat, rootId, scenarios.size, iae)
77-
}
78-
null
79-
}
80-
scenarios.forEach { it.rootLastRun = rootLastRun }
81-
}
82-
scenarios
83-
}
84-
}
8511

8612
fun Scenario.getRbac(): RbacSecurity {
8713
return RbacSecurity(

scenario/src/main/kotlin/com/cosmotech/scenario/service/ScenarioServiceImpl.kt

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -360,13 +360,11 @@ internal class ScenarioServiceImpl(
360360
var pageable = constructPageRequest(page, size, defaultPageSize)
361361
if (pageable != null) {
362362
return this.findPaginatedScenariosStateOption(
363-
organizationId, workspaceId, pageable.pageNumber, pageable.pageSize, true)
364-
.addLastRunsInfo(this, organizationId, workspaceId)
363+
organizationId, workspaceId, pageable.pageNumber, pageable.pageSize, true)
365364
}
366365
return findAllPaginated(defaultPageSize) {
367366
this.findPaginatedScenariosStateOption(
368367
organizationId, workspaceId, it.pageNumber, it.pageSize, true)
369-
.addLastRunsInfo(this, organizationId, workspaceId)
370368
.toMutableList()
371369
}
372370
}
@@ -496,9 +494,7 @@ internal class ScenarioServiceImpl(
496494
workspaceId: String,
497495
scenarioId: String
498496
): Scenario {
499-
val scenario =
500-
this.findScenarioByIdNoState(organizationId, workspaceId, scenarioId)
501-
.addLastRunsInfo(this, organizationId, workspaceId)
497+
val scenario = this.findScenarioByIdNoState(organizationId, workspaceId, scenarioId)
502498
csmRbac.verify(scenario.getRbac(), PERMISSION_READ, scenarioPermissions)
503499
this.addStateToScenario(organizationId, scenario)
504500
return scenario
@@ -604,8 +600,7 @@ internal class ScenarioServiceImpl(
604600
do {
605601
val scenarioList =
606602
this.findPaginatedScenariosStateOption(
607-
organizationId, workspaceId, pageable.pageNumber, pageable.pageSize, true)
608-
.addLastRunsInfo(this, organizationId, workspaceId)
603+
organizationId, workspaceId, pageable.pageNumber, pageable.pageSize, true)
609604
scenarioTree.addAll(scenarioList)
610605
pageable = pageable.next()
611606
} while (scenarioList.isNotEmpty())

scenario/src/test/kotlin/com/cosmotech/scenario/service/ScenarioServiceImplTests.kt

Lines changed: 2 additions & 172 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ import io.mockk.unmockkStatic
6161
import io.mockk.verify
6262
import java.util.Optional
6363
import java.util.UUID
64-
import java.util.stream.Stream
6564
import kotlin.test.AfterTest
6665
import kotlin.test.BeforeTest
6766
import kotlin.test.Ignore
@@ -75,11 +74,6 @@ import org.junit.jupiter.api.TestFactory
7574
import org.junit.jupiter.api.assertDoesNotThrow
7675
import org.junit.jupiter.api.assertThrows
7776
import org.junit.jupiter.api.extension.ExtendWith
78-
import org.junit.jupiter.api.extension.ExtensionContext
79-
import org.junit.jupiter.params.ParameterizedTest
80-
import org.junit.jupiter.params.provider.Arguments
81-
import org.junit.jupiter.params.provider.ArgumentsProvider
82-
import org.junit.jupiter.params.provider.ArgumentsSource
8377
import org.springframework.data.domain.PageImpl
8478

8579
const val ORGANIZATION_ID = "O-AbCdEf123"
@@ -756,172 +750,6 @@ class ScenarioServiceImplTests {
756750
}
757751
}
758752

759-
@Test
760-
fun `PROD-8051 - findAllScenarios add info about parent and master lastRuns`() {
761-
/* Scenario tree: M1 (run) -- (P11 (never run) -- C111 (run) */
762-
every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin")
763-
val m1 =
764-
Scenario(
765-
id = "M1",
766-
parentId = null,
767-
rootId = null,
768-
lastRun =
769-
ScenarioLastRun(
770-
scenarioRunId = "sr-m1",
771-
workflowName = "m1-workflowName",
772-
workflowId = "m1-workflowId",
773-
csmSimulationRun = "m1-csmSimulationRun"))
774-
val p11 = Scenario(id = "P11", parentId = m1.id, rootId = m1.id, lastRun = null)
775-
val c111 =
776-
Scenario(
777-
id = "C111",
778-
parentId = p11.id,
779-
rootId = m1.id,
780-
lastRun =
781-
ScenarioLastRun(
782-
scenarioRunId = "sr-c111",
783-
workflowName = "c111-workflowName",
784-
workflowId = "c111-workflowId",
785-
csmSimulationRun = "c111-csmSimulationRun"))
786-
val organization = Organization(id = ORGANIZATION_ID, security = null)
787-
val workspace = mockWorkspace(ORGANIZATION_ID, SOLUTION_ID, "WorkspaceName")
788-
every { csmPlatformProperties.twincache.scenario.defaultPageSize } returns 100
789-
every { organizationService.findOrganizationById(ORGANIZATION_ID) } returns organization
790-
every { workspaceService.findWorkspaceById(ORGANIZATION_ID, WORKSPACE_ID) } returns workspace
791-
every {
792-
scenarioServiceImpl.findScenarioByIdNoState(ORGANIZATION_ID, WORKSPACE_ID, m1.id!!)
793-
} returns m1
794-
every {
795-
scenarioServiceImpl.findScenarioByIdNoState(ORGANIZATION_ID, WORKSPACE_ID, p11.id!!)
796-
} returns p11
797-
every {
798-
scenarioServiceImpl.findScenarioByIdNoState(ORGANIZATION_ID, WORKSPACE_ID, c111.id!!)
799-
} returns c111
800-
every {
801-
scenarioServiceImpl.findPaginatedScenariosStateOption(
802-
ORGANIZATION_ID, WORKSPACE_ID, 0, 100, true)
803-
} returns listOf(m1, p11, c111)
804-
805-
val allScenariosById =
806-
scenarioServiceImpl
807-
.findAllScenarios(ORGANIZATION_ID, WORKSPACE_ID, 0, 100)
808-
.associateBy(Scenario::id)
809-
assertEquals(3, allScenariosById.size)
810-
assertTrue { allScenariosById.containsKey(m1.id) }
811-
assertTrue { allScenariosById.containsKey(p11.id) }
812-
assertTrue { allScenariosById.containsKey(c111.id) }
813-
814-
val m1Found = allScenariosById[m1.id]!!
815-
assertNull(m1Found.parentLastRun)
816-
assertNull(m1Found.rootLastRun)
817-
818-
val p11Found = allScenariosById[p11.id]!!
819-
assertEquals(m1Found.lastRun, p11Found.parentLastRun)
820-
assertEquals(m1Found.lastRun, p11Found.rootLastRun)
821-
822-
val c111Found = allScenariosById[c111.id]!!
823-
assertNull(c111Found.parentLastRun)
824-
assertEquals(m1Found.lastRun, c111Found.rootLastRun)
825-
}
826-
827-
@Test
828-
fun `PROD-8051 - findScenarioById adds null parent and master lastRuns if they don't exist`() {
829-
val parentId = "s-no-longer-existing-parent"
830-
val rootId = "s-no-longer-existing-root"
831-
val organization = Organization(id = ORGANIZATION_ID, security = null)
832-
val workspace =
833-
Workspace(
834-
id = WORKSPACE_ID,
835-
security = null,
836-
key = "w-myWorkspaceKey",
837-
name = "wonderful_workspace",
838-
solution = WorkspaceSolution(solutionId = "w-sol-id"))
839-
every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin")
840-
every { organizationService.findOrganizationById(ORGANIZATION_ID) } returns organization
841-
every { workspaceService.findWorkspaceById(ORGANIZATION_ID, WORKSPACE_ID) } returns workspace
842-
every {
843-
scenarioServiceImpl.findScenarioByIdNoState(ORGANIZATION_ID, WORKSPACE_ID, parentId)
844-
} throws CsmResourceNotFoundException("Scenario not found")
845-
every {
846-
scenarioServiceImpl.findScenarioByIdNoState(ORGANIZATION_ID, WORKSPACE_ID, rootId)
847-
} throws CsmResourceNotFoundException("Scenario not found")
848-
849-
val scenarioId = "s-c1"
850-
Scenario(id = scenarioId, parentId = parentId, rootId = rootId)
851-
val scenario = Scenario(id = scenarioId, parentId = parentId, rootId = rootId)
852-
every {
853-
scenarioServiceImpl.findScenarioByIdNoState(ORGANIZATION_ID, WORKSPACE_ID, scenarioId)
854-
} returns scenario
855-
856-
val scenarioReturned =
857-
scenarioServiceImpl.findScenarioById(ORGANIZATION_ID, WORKSPACE_ID, scenarioId)
858-
859-
assertNull(scenarioReturned.parentLastRun)
860-
assertNull(scenarioReturned.rootLastRun)
861-
}
862-
863-
@ParameterizedTest(name = "(parentId, rootId) = ({0}, {1})")
864-
@ArgumentsSource(ParentRootScenarioIdsCartesianProductArgumentsProvider::class)
865-
fun `PROD-8051 - findScenarioById adds info about parent and master lastRuns`(
866-
parentScenarioId: String?,
867-
rootScenarioId: String?
868-
) {
869-
val parentLastRun: ScenarioLastRun?
870-
if (!parentScenarioId.isNullOrBlank()) {
871-
parentLastRun =
872-
ScenarioLastRun(
873-
scenarioRunId = "sr-parentScenarioRunId",
874-
workflowName = "parent-workflowName",
875-
workflowId = "parent-workflowId",
876-
csmSimulationRun = "parent-csmSimulationRun")
877-
val parent = Scenario(id = parentScenarioId, lastRun = parentLastRun)
878-
every {
879-
scenarioServiceImpl.findScenarioByIdNoState(ORGANIZATION_ID, WORKSPACE_ID, parentScenarioId)
880-
} returns parent
881-
} else {
882-
parentLastRun = null
883-
}
884-
885-
val rootLastRun: ScenarioLastRun?
886-
if (!rootScenarioId.isNullOrBlank()) {
887-
rootLastRun =
888-
ScenarioLastRun(
889-
scenarioRunId = "sr-rootScenarioRunId",
890-
workflowName = "root-workflowName",
891-
workflowId = "root-workflowId",
892-
csmSimulationRun = "root-csmSimulationRun")
893-
val root = Scenario(id = rootScenarioId, lastRun = rootLastRun)
894-
every {
895-
scenarioServiceImpl.findScenarioByIdNoState(ORGANIZATION_ID, WORKSPACE_ID, rootScenarioId)
896-
} returns root
897-
} else {
898-
rootLastRun = null
899-
}
900-
901-
val scenarioId = "S-myScenarioId"
902-
val scenario = Scenario(id = scenarioId, parentId = parentScenarioId, rootId = rootScenarioId)
903-
val organization = Organization(id = ORGANIZATION_ID, security = null)
904-
val workspace =
905-
Workspace(
906-
id = WORKSPACE_ID,
907-
security = null,
908-
key = "w-myWorkspaceKey",
909-
name = "wonderful_workspace",
910-
solution = WorkspaceSolution(solutionId = "w-sol-id"))
911-
every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin")
912-
every { organizationService.findOrganizationById(ORGANIZATION_ID) } returns organization
913-
every { workspaceService.findWorkspaceById(ORGANIZATION_ID, WORKSPACE_ID) } returns workspace
914-
every {
915-
scenarioServiceImpl.findScenarioByIdNoState(ORGANIZATION_ID, WORKSPACE_ID, scenarioId)
916-
} returns scenario
917-
918-
val scenarioReturned =
919-
scenarioServiceImpl.findScenarioById(ORGANIZATION_ID, WORKSPACE_ID, scenarioId)
920-
921-
assertEquals(parentLastRun, scenarioReturned.parentLastRun)
922-
assertEquals(rootLastRun, scenarioReturned.rootLastRun)
923-
}
924-
925753
@Test
926754
fun `should test import Scenario method and assert it registered`() {
927755
val scenario = mockScenario()
@@ -1393,6 +1221,7 @@ class ScenarioServiceImplTests {
13931221
}
13941222
}
13951223

1224+
/*
13961225
private fun <T, U> List<T>.cartesianProduct(otherList: List<U>): List<Pair<T, U>> =
13971226
this.flatMap { elementInThisList ->
13981227
otherList.map { elementInOtherList -> elementInThisList to elementInOtherList }
@@ -1405,3 +1234,4 @@ private class ParentRootScenarioIdsCartesianProductArgumentsProvider : Arguments
14051234
.map { (parentId, rootId) -> Arguments.of(parentId, rootId) }
14061235
.stream()
14071236
}
1237+
*/

0 commit comments

Comments
 (0)