Reduce model and result coupling (#72)

web/status-history/src/main/java/net/twisterrob/travel/statushistory/controller/LineStatusHistoryController.kt

@@ -10,13 +10,13 @@ import io.micronaut.views.View
 import net.twisterrob.blt.data.StaticData
 import net.twisterrob.blt.io.feeds.trackernet.LineStatusFeed
 import net.twisterrob.travel.domain.london.status.Feed
-import net.twisterrob.travel.domain.london.status.api.StatusHistoryRepository
 import net.twisterrob.travel.domain.london.status.api.ParsedStatusItem
+import net.twisterrob.travel.domain.london.status.api.StatusHistoryRepository
 import net.twisterrob.travel.statushistory.viewmodel.LineColor
+import net.twisterrob.travel.statushistory.viewmodel.LineStatusModel
 import net.twisterrob.travel.statushistory.viewmodel.Result
-import net.twisterrob.travel.statushistory.viewmodel.ResultChangesCalculator
-import net.twisterrob.travel.statushistory.viewmodel.ResultChangeModel
 import net.twisterrob.travel.statushistory.viewmodel.ResultChangeModelMapper
+import net.twisterrob.travel.statushistory.viewmodel.ResultChangesCalculator
 import java.util.Date
 
 @Controller
@@ -41,18 +41,12 @@ class LineStatusHistoryController(
 		val changes = ResultChangesCalculator().getChanges(results)
 
 		return HttpResponse.ok(
-			LineStatusHistoryModel(
+			LineStatusModel(
 				changes.map(ResultChangeModelMapper()::map),
 				LineColor.AllColors(staticData.lineColors)
 			)
 		)
 	}
-
-	@Suppress("unused") // Used by LineStatus.hbs.
-	private class LineStatusHistoryModel(
-		val feedChanges: List<ResultChangeModel>,
-		val colors: Iterable<LineColor>,
-	)
 }
 
 private fun ParsedStatusItem.toResult(): Result {

web/status-history/src/main/java/net/twisterrob/travel/statushistory/viewmodel/Changes.kt

@@ -6,7 +6,7 @@ import net.twisterrob.blt.model.Line
 
 sealed interface Changes {
 
-	data object None : Changes
+	data object Inconclusive : Changes
 
 	data class NewStatus(
 		val current: Result,

web/status-history/src/main/java/net/twisterrob/travel/statushistory/viewmodel/Result.kt

@@ -5,7 +5,6 @@ import java.util.Date
 
 sealed interface Result {
 
-	// Used by Handlebars reflection in LineStatus.hbs.
 	@Suppress("detekt.VariableNaming")
 	val `when`: Date
 
@@ -21,11 +20,11 @@ sealed interface Result {
 
 		@JvmInline
 		value class Error(
-			val error: String,
+			val text: String,
 		) {
 
 			val header: String
-				get() = error.substringBefore('\n')
+				get() = text.substringBefore('\n')
 		}
 	}
 }

web/status-history/src/main/java/net/twisterrob/travel/statushistory/viewmodel/ResultChangeModel.kt

@@ -1,15 +1,32 @@
+@file:Suppress("unused", "CanBeParameter", "MemberVisibilityCanBePrivate") // Used by LineStatus.hbs.
+
 package net.twisterrob.travel.statushistory.viewmodel
 
+import net.twisterrob.blt.io.feeds.trackernet.model.DelayType
 import net.twisterrob.blt.model.Line
+import java.util.Date
+
+class LineStatusModel(
+	val feedChanges: List<ResultChangeModel>,
+	val colors: Iterable<LineColor>,
+)
 
-open class ResultChangeModel(
-	val previous: Result?,
-	val current: Result?,
+class ResultChangeModel(
+	val `when`: Date?,
+	val statuses: List<LineStatusModel>,
 	val error: ErrorChange?,
-	val statuses: Map<Line, StatusChange>,
-	val descriptions: Map<Line, String>,
 ) {
 
+	class LineStatusModel(
+		val line: Line,
+		val type: DelayType,
+		val description: String?,
+		val active: Boolean,
+		val branchDescription: String?,
+		val changeStatus: StatusChange?,
+		val changeDescription: String?,
+	)
+
 	enum class StatusChange(
 		val title: String,
 		val cssClass: String,
@@ -24,18 +41,30 @@ open class ResultChangeModel(
 		SameDescriptionAdd("+ descr.", "status-same-desc-add"),
 		SameDescriptionDel("- descr.", "status-same-desc-del"),
 		BranchesChange("branches", "status-same-branch-change"),
+		;
+
+		init {
+			require(cssClass.isNotEmpty()) { "CSS class must not be empty: ${this} has cssClass=${cssClass}." }
+		}
 	}
 
-	enum class ErrorChange(
-		val title: String,
+	class ErrorChange(
+		val type: Type,
+		val header: String?,
+		val full: String?,
 	) {
 
-		Same("same error"),
-		Change("error changed"),
-		Failed("new error"),
-		Fixed("error fixed"),
-		NoErrors(""),
-		NewStatus(""),
-		LastStatus(""),
+		enum class Type(
+			val title: String,
+		) {
+
+			Same("same error"),
+			Change("error changed"),
+			Failed("new error"),
+			Fixed("error fixed"),
+			NoErrors(""),
+			NewStatus(""),
+			LastStatus(""),
+		}
 	}
 }

web/status-history/src/main/java/net/twisterrob/travel/statushistory/viewmodel/ResultChangeModelMapper.kt

@@ -2,33 +2,40 @@ package net.twisterrob.travel.statushistory.viewmodel
 
 import net.twisterrob.blt.diff.HtmlDiff
 import net.twisterrob.blt.io.feeds.trackernet.model.LineStatus
+import net.twisterrob.blt.model.Line
+import net.twisterrob.travel.statushistory.viewmodel.ResultChangeModel.LineStatusModel
 
 class ResultChangeModelMapper {
 
-	fun map(changes: Changes): ResultChangeModel {
-		val statusChanges = (changes as? Changes.Status)?.changes.orEmpty()
-		return ResultChangeModel(
-			previous = changes.previous,
-			current = changes.current,
+	fun map(changes: Changes): ResultChangeModel =
+		ResultChangeModel(
+			`when` = changes.current?.`when`,
 			error = mapError(changes),
-			statuses = statusChanges.mapValues { map(it.value) },
-			descriptions = statusChanges
-				.filterValues { it is HasDescriptionChange }
-				.mapValues { it.value as HasDescriptionChange }
-				.mapValues { diffDesc(it.value.desc) },
+			statuses = map(
+				(changes.current as? Result.ContentResult)?.content?.lineStatuses.orEmpty(),
+				(changes as? Changes.Status)?.changes.orEmpty(),
+			),
+		)
+
+	private fun mapError(changes: Changes): ResultChangeModel.ErrorChange {
+		val error = (changes.current as? Result.ErrorResult)?.error
+		return ResultChangeModel.ErrorChange(
+			full = error?.text,
+			header = error?.header,
+			type = mapErrorType(changes),
 		)
 	}
 
-	private fun mapError(changes: Changes): ResultChangeModel.ErrorChange =
+	private fun mapErrorType(changes: Changes): ResultChangeModel.ErrorChange.Type =
 		when (changes) {
-			is Changes.Status -> ResultChangeModel.ErrorChange.NoErrors
-			Changes.None -> ResultChangeModel.ErrorChange.NoErrors
-			is Changes.NewStatus -> ResultChangeModel.ErrorChange.NewStatus
-			is Changes.LastStatus -> ResultChangeModel.ErrorChange.LastStatus
-			is Changes.ErrorChanges.Same -> ResultChangeModel.ErrorChange.Same
-			is Changes.ErrorChanges.Change -> ResultChangeModel.ErrorChange.Change
-			is Changes.ErrorChanges.Failed -> ResultChangeModel.ErrorChange.Failed
-			is Changes.ErrorChanges.Fixed -> ResultChangeModel.ErrorChange.Fixed
+			is Changes.Status -> ResultChangeModel.ErrorChange.Type.NoErrors
+			Changes.Inconclusive -> ResultChangeModel.ErrorChange.Type.NoErrors
+			is Changes.NewStatus -> ResultChangeModel.ErrorChange.Type.NewStatus
+			is Changes.LastStatus -> ResultChangeModel.ErrorChange.Type.LastStatus
+			is Changes.ErrorChanges.Same -> ResultChangeModel.ErrorChange.Type.Same
+			is Changes.ErrorChanges.Change -> ResultChangeModel.ErrorChange.Type.Change
+			is Changes.ErrorChanges.Failed -> ResultChangeModel.ErrorChange.Type.Failed
+			is Changes.ErrorChanges.Fixed -> ResultChangeModel.ErrorChange.Type.Fixed
 		}
 
 	private fun map(value: StatusChange): ResultChangeModel.StatusChange =
@@ -57,26 +64,26 @@ class ResultChangeModelMapper {
 			DescriptionChange.Missing -> ""
 		}
 
-	private val Changes.previous: Result?
-		get() =
-			when (this) {
-				is Changes.Status -> previous
-				is Changes.NewStatus -> null
-				is Changes.LastStatus -> previous
-				is Changes.None -> null
-				is Changes.ErrorChanges.Change -> oldError
-				is Changes.ErrorChanges.Failed -> null
-				is Changes.ErrorChanges.Fixed -> oldError
-				is Changes.ErrorChanges.Same -> error
-			}
+	private fun map(statuses: List<LineStatus>, changes: Map<Line, StatusChange>): List<LineStatusModel> =
+		statuses.map { lineStatus ->
+			LineStatusModel(
+				line = lineStatus.line,
+				type = lineStatus.type,
+				description = lineStatus.description,
+				changeStatus = changes[lineStatus.line]?.let(::map),
+				changeDescription = (changes[lineStatus.line] as? HasDescriptionChange)?.let { diffDesc(it.desc) },
+				active = lineStatus.isActive,
+				branchDescription = describe(lineStatus.branchStatuses),
+			)
+		}
 
 	private val Changes.current: Result?
 		get() =
 			when (this) {
 				is Changes.Status -> current
 				is Changes.NewStatus -> current
 				is Changes.LastStatus -> null
-				is Changes.None -> null
+				is Changes.Inconclusive -> null
 				is Changes.ErrorChanges.Change -> newError
 				is Changes.ErrorChanges.Failed -> newError
 				is Changes.ErrorChanges.Fixed -> newResult

web/status-history/src/main/java/net/twisterrob/travel/statushistory/viewmodel/ResultChangesCalculator.kt

@@ -24,7 +24,7 @@ class ResultChangesCalculator {
 			oldResult == null && newResult != null -> Changes.NewStatus(newResult)
 			oldResult != null && newResult == null -> Changes.LastStatus(oldResult)
 			oldResult != null && newResult != null -> diffResults(oldResult, newResult)
-			else /* oldResult == null && newResult == null */ -> Changes.None
+			else /* oldResult == null && newResult == null */ -> Changes.Inconclusive
 		}
 
 	private fun diffResults(oldResult: Result, newResult: Result): Changes =

web/status-history/src/main/resources/views/LineStatus.hbs

@@ -71,51 +71,50 @@
 </head>
 <body>
 {{#each feedChanges as | feedChange | }}
-	{{assign "feed" feedChange.current}}
 	<table class="info">
 		<thead>
 		<tr>
 			<th colspan="3" class="title">
 				{{add @index 1}}/{{../feedChanges.length}}:
-				{{formatDate feed.when "yyyy-MM-dd HH:mm:ss"}}
+				{{formatDate feedChange.when "yyyy-MM-dd HH:mm:ss"}}
 			</th>
 		</tr>
+		{{#if (empty feedChange.error.header)}}
 		<tr>
 			<th>Line</th>
 			<th>Disruption Type</th>
 			<th>Change</th>
 		</tr>
+		{{/if}}
 		</thead>
 		<tbody>
-		{{#each feed.content.lineStatuses as | lineStatus | }}
-			{{assign "changeStatus" (lookupMap feedChange.statuses lineStatus.line)}}
-			{{assign "changeDescription" (lookupMap feedChange.descriptions lineStatus.line)}}
+		{{#each feedChange.statuses as | status | }}
 			{{assign "delayStyle" ""}}
-			{{#if (not (empty lineStatus.description))}}
+			{{#if (not (empty status.description))}}
 				{{assign "delayStyle" (concat delayStyle " hasDetails")}}
 			{{/if}}
-			{{#if (not lineStatus.active)}}
+			{{#if (not status.active)}}
 				{{assign "delayStyle" (concat delayStyle " inactive")}}
 			{{/if}}
 			<tr>
-				<td class="line-{{lineStatus.line}}">{{lineStatus.line.title}}</td>
-				<td class="delay {{changeStatus.cssClass}} {{delayStyle}}">
-					{{#if (or (empty lineStatus.description) (empty lineStatus.branchDescription))}}
-						{{lineStatus.type.title}}
+				<td class="line-{{status.line}}">{{status.line.title}}</td>
+				<td class="delay {{status.changeStatus.cssClass}} {{delayStyle}}">
+					{{#if (or (empty status.description) (empty status.branchDescription))}}
+						{{status.type.title}}
 					{{else}}
-						<a rel="htmltooltip">{{lineStatus.type.title}}</a>
+						<a rel="htmltooltip">{{status.type.title}}</a>
 						<div class="htmltooltip">
-							<p>{{lineStatus.description}}</p>
-							<p>{{lineStatus.branchDescription}}</p>
+							<p>{{status.description}}</p>
+							<p>{{status.branchDescription}}</p>
 						</div>
 					{{/if}}
 				</td>
-				<td class="change {{changeStatus.cssClass}}">
-					{{#if (empty changeDescription)}}
-						{{changeStatus.title}}
+				<td class="change {{status.changeStatus.cssClass}}">
+					{{#if (empty status.changeDescription)}}
+						{{status.changeStatus.title}}
 					{{else}}
-						<a rel="htmltooltip">{{changeStatus.title}}</a>
-						<div class="htmltooltip">{{{changeDescription}}}</div>
+						<a rel="htmltooltip">{{status.changeStatus.title}}</a>
+						<div class="htmltooltip">{{{status.changeDescription}}}</div>
 					{{/if}}
 				</td>
 			</tr>
@@ -124,13 +123,13 @@
 		<tfoot>
 		<tr>
 			<td colspan="3">
-				{{#if (not (empty feed.errorHeader))}}
-					<a rel="htmltooltip">{{feedChange.error.title}}: {{feed.errorHeader}}</a>
+				{{#if (not (empty feedChange.error.header))}}
+					<a rel="htmltooltip">{{feedChange.error.type.title}}: {{feedChange.error.header}}</a>
 					<div class="htmltooltip">
-						<pre style="font-size: xx-small;">{{feed.fullError}}</pre>
+						<pre style="font-size: xx-small;">{{feedChange.error.full}}</pre>
 					</div>
-				{{else if (not (empty feedChange.error.title))}}
-					{{feedChange.error.title}}
+				{{else if (not (empty feedChange.error.type.title))}}
+					{{feedChange.error.type.title}}
 				{{else}}
 					{{!-- Make sure there's always content, otherwise the height of the tables is not the same --}}
 					&nbsp;

web/status-history/src/test/java/net/twisterrob/travel/statushistory/viewmodel/ResultChangeCalculatorUnitTest_NonContentDiffs.kt

@@ -91,7 +91,7 @@ class ResultChangeCalculatorUnitTest_NonContentDiffs {
 	@Test fun testMissingResults() {
 		val difference = subject.diff(null, null)
 
-		assertEquals(difference, Changes.None)
+		assertEquals(difference, Changes.Inconclusive)
 	}
 }