From d47eea56a057b94ff1f19d815fed6f693c0c72b8 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 26 Jun 2023 02:21:05 +0000 Subject: [PATCH] Fix timing point validation. There was a bug where it was only validated if the first and the last stop point have a timing place association. The fix involves validating that the first and the last stop point are used as timing points in the journey pattern. Resolves HSLdevcom/jore4#1339 --- pom.xml | 8 + .../hsl/jore4/hastus/export/ExportService.kt | 17 +- .../jore4/hastus/export/ExportServiceTest.kt | 245 ++++++++++++++++++ 3 files changed, 264 insertions(+), 6 deletions(-) create mode 100644 src/test/kotlin/fi/hsl/jore4/hastus/export/ExportServiceTest.kt diff --git a/pom.xml b/pom.xml index fc4a52c8..443abb97 100644 --- a/pom.xml +++ b/pom.xml @@ -32,6 +32,7 @@ 6.4.1 2.0.3 2.15.1 + 1.12.4 fi.hsl.jore4.hastus.HastusApplicationKt @@ -536,6 +537,13 @@ test + + io.mockk + mockk + ${mockk.version} + test + + org.quicktheories quicktheories diff --git a/src/main/kotlin/fi/hsl/jore4/hastus/export/ExportService.kt b/src/main/kotlin/fi/hsl/jore4/hastus/export/ExportService.kt index 1fd2c8af..96e12175 100644 --- a/src/main/kotlin/fi/hsl/jore4/hastus/export/ExportService.kt +++ b/src/main/kotlin/fi/hsl/jore4/hastus/export/ExportService.kt @@ -3,6 +3,7 @@ package fi.hsl.jore4.hastus.export import fi.hsl.jore4.hastus.data.hastus.IHastusData import fi.hsl.jore4.hastus.data.jore.JoreDistanceBetweenTwoStopPoints import fi.hsl.jore4.hastus.data.jore.JoreLine +import fi.hsl.jore4.hastus.data.jore.JoreRouteScheduledStop import fi.hsl.jore4.hastus.data.jore.JoreScheduledStop import fi.hsl.jore4.hastus.data.jore.JoreTimingPlace import fi.hsl.jore4.hastus.data.mapper.ConversionsToHastus @@ -80,20 +81,24 @@ class ExportService @Autowired constructor( } } - if (route.stopsOnRoute.first().timingPlaceShortName == null) { + val firstStopOnRoute: JoreRouteScheduledStop = route.stopsOnRoute.first() + + if (!firstStopOnRoute.isTimingPoint || firstStopOnRoute.timingPlaceShortName == null) { LOGGER.warn { - "The first stop point of the journey pattern for route ${route.label} is not a timing " + - "point as mandated by Hastus" + "The first stop point of the journey pattern for route ${route.label} is not a valid " + + "timing point as mandated by Hastus" } if (failOnTimingPointValidation) { throw FirstStopNotTimingPointException(route.label) } } - if (route.stopsOnRoute.last().timingPlaceShortName == null) { + val lastStopOnRoute: JoreRouteScheduledStop = route.stopsOnRoute.last() + + if (!lastStopOnRoute.isTimingPoint || lastStopOnRoute.timingPlaceShortName == null) { LOGGER.warn { - "The last stop point of the journey pattern for route ${route.label} is not a timing " + - "point as mandated by Hastus" + "The last stop point of the journey pattern for route ${route.label} is not a valid " + + "timing point as mandated by Hastus" } if (failOnTimingPointValidation) { throw LastStopNotTimingPointException(route.label) diff --git a/src/test/kotlin/fi/hsl/jore4/hastus/export/ExportServiceTest.kt b/src/test/kotlin/fi/hsl/jore4/hastus/export/ExportServiceTest.kt new file mode 100644 index 00000000..73910739 --- /dev/null +++ b/src/test/kotlin/fi/hsl/jore4/hastus/export/ExportServiceTest.kt @@ -0,0 +1,245 @@ +package fi.hsl.jore4.hastus.export + +import fi.hsl.jore4.hastus.data.jore.JoreLine +import fi.hsl.jore4.hastus.data.jore.JoreRoute +import fi.hsl.jore4.hastus.data.jore.JoreRouteScheduledStop +import fi.hsl.jore4.hastus.graphql.FetchRoutesResult +import fi.hsl.jore4.hastus.graphql.GraphQLService +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.junit5.MockKExtension +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import java.time.LocalDate +import kotlin.test.assertFailsWith + +@ExtendWith(MockKExtension::class) +class ExportServiceTest { + + @MockK + lateinit var graphQLService: GraphQLService + + lateinit var exportService: ExportService + + @BeforeEach + fun setupServiceUnderTest() { + exportService = ExportService(graphQLService, true) + } + + @DisplayName("Validate deep-fetched routes got from GraphQLService.deepFetchRoutes(...)") + @Nested + inner class ValidateDeepFetchedRoutes { + + private fun stubDeepFetchRoutesForValidationSideEffects(line: JoreLine): FetchRoutesResult { + val fetchRoutesResult = FetchRoutesResult(listOf(line), emptyList(), emptyList(), emptyList()) + + // given + every { + graphQLService.deepFetchRoutes(any(), any(), any(), any()) + } /* then */ returns fetchRoutesResult + + return fetchRoutesResult + } + + private fun invokeExportRoutesWithAnyParameters() { + // Because of the stubbing done in stubDeepFetchRoutesForValidationSideEffects() the + // parameter values used here are not meaningful. Anything goes. + exportService.exportRoutes(listOf(), 10, LocalDate.now(), emptyMap()) + } + + @DisplayName("Validation should succeed when the first and the last stop points are timing points") + @Test + fun smoke() { + val stopPoints = listOf( + createFirstStopPoint("1KALA"), + createFirstStopPoint("1ELIEL") + ) + val line = createLine(stopPoints) + + stubDeepFetchRoutesForValidationSideEffects(line) + + invokeExportRoutesWithAnyParameters() + } + + @DisplayName("When the journey pattern consists of less than two stop points") + @Nested + inner class WhenThereAreLessThanTwoStopPoints { + + @DisplayName("When there is only one stop point in journey pattern") + @Test + fun whenFirstStopPointIsNotTimingPointAndDoesNotHaveTimingPlaceAssociation() { + val line = createLine( + listOf( + createFirstStopPoint("1KALA") + // no other stop points given, just one + ) + ) + + stubDeepFetchRoutesForValidationSideEffects(line) + + assertFailsWith { + invokeExportRoutesWithAnyParameters() + } + } + } + + @DisplayName("When the first stop point in journey pattern is not a valid timing point") + @Nested + inner class WhenFirstStopPointIsNotTimingPoint { + + @DisplayName("When the first stop point is not a timing point and does not have timing place name") + @Test + fun whenFirstStopPointIsNotTimingPointAndDoesNotHaveTimingPlaceName() { + val stopPoints = listOf( + createFirstStopPoint(null, false), + createLastStopPoint("1ELIEL") + ) + val line = createLine(stopPoints) + + stubDeepFetchRoutesForValidationSideEffects(line) + + assertFailsWith { + invokeExportRoutesWithAnyParameters() + } + } + + @DisplayName("When the first stop point is a timing point but does not have timing place name") + @Test + fun whenFirstStopPointIsTimingPointButDoesNotHaveTimingPlaceName() { + val stopPoints = listOf( + createFirstStopPoint(null, true), + createLastStopPoint("1ELIEL") + ) + val line = createLine(stopPoints) + + stubDeepFetchRoutesForValidationSideEffects(line) + + assertFailsWith { + invokeExportRoutesWithAnyParameters() + } + } + + @DisplayName("When the first stop point is not a timing point but has timing place name") + @Test + fun whenFirstStopPointIsNotTimingPointButHasTimingPlaceName() { + val stopPoints = listOf( + createFirstStopPoint("1KALA", false), + createLastStopPoint("1ELIEL") + ) + val line = createLine(stopPoints) + + stubDeepFetchRoutesForValidationSideEffects(line) + + assertFailsWith { + invokeExportRoutesWithAnyParameters() + } + } + } + + @DisplayName("When the last stop point in journey pattern is not a valid timing point") + @Nested + inner class WhenLastStopPointIsNotTimingPoint { + + @DisplayName("When the last stop point is not a timing point and does not have timing place name") + @Test + fun whenLastStopPointIsNotTimingPointAndDoesNotHaveTimingPlaceName() { + val stopPoints = listOf( + createFirstStopPoint("1KALA"), + createLastStopPoint(null, false) + ) + val line = createLine(stopPoints) + + stubDeepFetchRoutesForValidationSideEffects(line) + + assertFailsWith { + invokeExportRoutesWithAnyParameters() + } + } + + @DisplayName("When the last stop point is a timing point but does not have timing place name") + @Test + fun whenLastStopPointIsTimingPointButDoesNotHaveTimingPlaceName() { + val stopPoints = listOf( + createFirstStopPoint("1ELIEL"), + createLastStopPoint(null, true) + ) + val line = createLine(stopPoints) + + stubDeepFetchRoutesForValidationSideEffects(line) + + assertFailsWith { + invokeExportRoutesWithAnyParameters() + } + } + + @DisplayName("When the last stop point is not a timing point but has timing place name") + @Test + fun whenLastStopPointIsNotTimingPointButHasTimingPlaceName() { + val stopPoints = listOf( + createFirstStopPoint("1KALA"), + createLastStopPoint("1ELIEL", false) + ) + val line = createLine(stopPoints) + + stubDeepFetchRoutesForValidationSideEffects(line) + + assertFailsWith { + invokeExportRoutesWithAnyParameters() + } + } + } + } + + companion object { + + fun createLine(stopsOnRoute: List): JoreLine { + return JoreLine( + label = "65", + "Rautatientori - Veräjälaakso FI", + 0, + listOf( + JoreRoute( + label = "65x", + variant = "", + uniqueLabel = "65x", + name = "Reitti A - B FI", + direction = 1, + reversible = false, + stopsOnRoute = stopsOnRoute + ) + ) + ) + } + + fun createFirstStopPoint( + timingPlaceShortName: String?, + isTimingPoint: Boolean = true + ): JoreRouteScheduledStop { + return JoreRouteScheduledStop( + timingPlaceShortName = timingPlaceShortName, + distanceToNextStop = 123.0, + isRegulatedTimingPoint = false, + isAllowedLoad = false, + isTimingPoint = isTimingPoint, + stopLabel = "H1000" + ) + } + + fun createLastStopPoint( + timingPlaceShortName: String?, + isTimingPoint: Boolean = true + ): JoreRouteScheduledStop { + return JoreRouteScheduledStop( + timingPlaceShortName = timingPlaceShortName, + distanceToNextStop = 0.0, + isRegulatedTimingPoint = false, + isAllowedLoad = false, + isTimingPoint = isTimingPoint, + stopLabel = "H9999" + ) + } + } +}