From f6698a360ad95ac78223ed75656542148bbebe3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mark=20Hu=CC=88sers?= Date: Thu, 19 Oct 2023 17:24:24 +0200 Subject: [PATCH] chore: Add operators for generated VSS models These add a lot of convenience to the usage of manipulating values inside these models. No one has to remember the correct copy method now. --- .../view/DataBrokerConnectionView.kt | 4 +- .../extension/compose/CountdownView.kt | 2 +- .../org/eclipse/kuksa/DataBrokerConnection.kt | 12 +-- .../VssSpecificationCopyExtension.kt | 84 ++++++++++++------ .../kuksa/vssSpecification/VssDriver.kt | 86 ++++++++++++++++--- .../VssSpecificationCopyTest.kt | 65 +++++++++++--- .../kuksa/vsscore/model/VssSpecification.kt | 20 ++++- 7 files changed, 215 insertions(+), 58 deletions(-) diff --git a/app/src/main/kotlin/org/eclipse/kuksa/testapp/databroker/view/DataBrokerConnectionView.kt b/app/src/main/kotlin/org/eclipse/kuksa/testapp/databroker/view/DataBrokerConnectionView.kt index 1813bbef..68ba2e0c 100644 --- a/app/src/main/kotlin/org/eclipse/kuksa/testapp/databroker/view/DataBrokerConnectionView.kt +++ b/app/src/main/kotlin/org/eclipse/kuksa/testapp/databroker/view/DataBrokerConnectionView.kt @@ -54,7 +54,7 @@ import org.eclipse.kuksa.testapp.databroker.model.ConnectionInfo import org.eclipse.kuksa.testapp.databroker.viewmodel.ConnectionViewModel import org.eclipse.kuksa.testapp.databroker.viewmodel.ConnectionViewModel.* import org.eclipse.kuksa.testapp.extension.compose.Headline -import org.eclipse.kuksa.testapp.extension.compose.rememberCountdown +import org.eclipse.kuksa.testapp.extension.compose.RememberCountdown import org.eclipse.kuksa.testapp.extension.fetchFileName import org.eclipse.kuksa.testapp.preferences.ConnectionInfoRepository @@ -216,7 +216,7 @@ fun DataBrokerConnection(viewModel: ConnectionViewModel) { onClick = { }, modifier = Modifier.requiredWidth(MinimumButtonWidth), ) { - val timeout by rememberCountdown(initialMillis = viewModel.connectionTimeoutMillis) + val timeout by RememberCountdown(initialMillis = viewModel.connectionTimeoutMillis) @Suppress("MagicNumber") // To seconds val timeoutSeconds = timeout / 1000 diff --git a/app/src/main/kotlin/org/eclipse/kuksa/testapp/extension/compose/CountdownView.kt b/app/src/main/kotlin/org/eclipse/kuksa/testapp/extension/compose/CountdownView.kt index f02d5d2b..a55bc41a 100644 --- a/app/src/main/kotlin/org/eclipse/kuksa/testapp/extension/compose/CountdownView.kt +++ b/app/src/main/kotlin/org/eclipse/kuksa/testapp/extension/compose/CountdownView.kt @@ -28,7 +28,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.isActive @Composable -fun rememberCountdown( +fun RememberCountdown( initialMillis: Long, step: Long = 1000, ): MutableState { diff --git a/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/DataBrokerConnection.kt b/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/DataBrokerConnection.kt index 3adc79f7..4a869ef9 100644 --- a/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/DataBrokerConnection.kt +++ b/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/DataBrokerConnection.kt @@ -165,7 +165,7 @@ class DataBrokerConnection internal constructor( initialSubscriptionUpdates[vssPath] = true val isInitialSubscriptionComplete = initialSubscriptionUpdates.values.all { it } if (isInitialSubscriptionComplete) { - Log.d(TAG, "Initial update for subscribed property complete: $vssPath - $updatedValue") + Log.d(TAG, "Update for subscribed property complete: $vssPath - $updatedValue") observer.onSpecificationChanged(updatedVssSpecification) } } @@ -252,14 +252,14 @@ class DataBrokerConnection internal constructor( */ suspend fun update( property: Property, - updatedDatapoint: Datapoint, + datapoint: Datapoint, ): SetResponse { - Log.d(TAG, "updateProperty() called with: updatedProperty = $property") + Log.d(TAG, "updateProperty() called with: updatedProperty = $property, datapoint: $datapoint") return withContext(dispatcher) { val blockingStub = VALGrpc.newBlockingStub(managedChannel) val dataEntry = Types.DataEntry.newBuilder() .setPath(property.vssPath) - .setValue(updatedDatapoint) + .setValue(datapoint) .build() val entryUpdate = KuksaValV1.EntryUpdate.newBuilder() .setEntry(dataEntry) @@ -280,8 +280,8 @@ class DataBrokerConnection internal constructor( /** * Only a [VssProperty] can be updated because they have an actual value. When provided with any parent * [VssSpecification] then this [update] method will find all [VssProperty] children and updates them instead. - * Compared to [update] with only one [Property] and [Datapoint], here multiple [SetResponse] will be returned - * because a [VssSpecification] may consists of multiple values which may need to be updated. + * Compared to [update] with only one [Property] and [Datapoint], here multiple [SetResponse] will be returned + * because a [VssSpecification] may consists of multiple values which may need to be updated. * * @throws DataBrokerException in case the connection to the DataBroker is no longer active */ diff --git a/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/VssSpecificationCopyExtension.kt b/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/VssSpecificationCopyExtension.kt index 7b1ef2e4..37d93db9 100644 --- a/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/VssSpecificationCopyExtension.kt +++ b/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/VssSpecificationCopyExtension.kt @@ -38,18 +38,26 @@ import org.eclipse.kuksa.vsscore.model.variableName * ``` * * A deep copy is necessary for a nested history tree with at least two generations. The VssWindowChildLockEngaged - * is replaced inside VssCabin where this again is replaced inside VssVehicle. - * - * @param generation the generation to start copying with starting from the [VssSpecification] to [deepCopy] - * @return a copy where every heir in the given [changedHeritageLine] is replaced with a another copy + * is replaced inside VssCabin where this again is replaced inside VssVehicle. Use the [generation] to start copying + * from the [VssSpecification] to the [deepCopy]. Returns a copy where every heir in the given [changedHeritage] is + * replaced with a another copy */ -fun T.deepCopy(changedHeritageLine: List, generation: Int = 0): T { - if (generation == changedHeritageLine.size) { // Reached the end, use the changed VssProperty +@Suppress("performance:SpreadOperator") +fun T.deepCopy(generation: Int = 0, vararg changedHeritage: VssSpecification): T { + if (generation == changedHeritage.size) { // Reached the end, use the changed VssProperty return this } - val childSpecification = changedHeritageLine[generation] - val childCopy = childSpecification.deepCopy(changedHeritageLine, generation + 1) + // Create the missing link between this (VssSpecification) and the given property (VssSpecifications inbetween) + var heritageLine = changedHeritage + if (changedHeritage.size == 1) { + heritageLine = findHeritageLine(changedHeritage.first(), true) + .toTypedArray() + .ifEmpty { changedHeritage } + } + + val childSpecification = heritageLine[generation] + val childCopy = childSpecification.deepCopy(generation + 1, *heritageLine) val parameterNameToChild = mapOf(childSpecification.variableName to childCopy) return copy(parameterNameToChild) @@ -58,12 +66,10 @@ fun T.deepCopy(changedHeritageLine: List VssProperty.copy(datapoint: Datapoint): VssProperty { with(datapoint) { val value: Any = when (valueCase) { @@ -106,21 +112,26 @@ fun VssProperty.copy(datapoint: Datapoint): VssProperty { null -> throw NoSuchFieldException("Could not convert available value: $value to type: ${value::class}") } - val valueMap = mapOf("value" to value) - return this@copy.copy(valueMap) + // Value must be T + return this@copy.copy(value as T) } } +/** + * Calls the generated copy method of the data class for the [VssProperty] and returns a new copy with the new [value]. + */ +fun VssProperty.copy(value: T): VssProperty { + val valueMap = mapOf("value" to value) + return this@copy.copy(valueMap) +} + /** * Creates a copy of the [VssSpecification] where the heir with a matching [vssPath] is replaced with the * [updatedValue]. * - * @param vssPath which is used to find the correct heir in the [VssSpecification] - * @param updatedValue which will be updated inside the matching [VssProperty] * @param consideredHeritage the heritage of the [VssSpecification] which is considered for searching. The default * will always generate the up to date heritage of the current [VssSpecification]. For performance reason it may make * sense to cache the input and reuse the [Collection] here. - * @return a copy where the heir with the matching [vssPath] is replaced with a another copy * * @throws [NoSuchElementException] if the class has no "copy" method * @throws [IllegalArgumentException] if the copied types do not match @@ -137,17 +148,38 @@ fun T.copy( .find { it.vssPath == vssPath } ?: return this val updatedVssProperty = vssProperty.copy(updatedValue) - val relevantChildren = findHeritageLine(vssProperty).toMutableList() - // Replace the last specification (Property) with the changed one - val updatedVssSpecification: T = if (relevantChildren.isNotEmpty()) { - relevantChildren.removeLast() - relevantChildren.add(updatedVssProperty) + // Same property with no heirs, no deep copy is needed + if (this.vssPath == updatedVssProperty.vssPath) return updatedVssProperty as T - this.deepCopy(relevantChildren) - } else { - updatedVssProperty as T // The property must be T since no children are available - } + return deepCopy(0, updatedVssProperty) +} - return updatedVssSpecification +/** + * Convenience operator for [copy] with a value [T]. + */ +operator fun VssProperty.plusAssign(value: T) { + copy(value) +} + +/** + * Convenience operator for [copy] with a value [T]. + */ +operator fun VssProperty.plus(value: T): VssProperty { + return copy(value) +} + +/** + * Convenience operator for [copy] with a [Boolean] value which will be inverted. + */ +operator fun VssProperty.not(): VssProperty { + return copy(!value) +} + +/** + * Convenience operator for [deepCopy] with a [VssProperty]. It will return the [VssSpecification] with the updated + * [VssProperty]. + */ +operator fun T.plus(property: VssProperty): T { + return deepCopy(0, property) } diff --git a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssSpecification/VssDriver.kt b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssSpecification/VssDriver.kt index 16942c78..cde2bf20 100644 --- a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssSpecification/VssDriver.kt +++ b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssSpecification/VssDriver.kt @@ -25,8 +25,34 @@ import org.eclipse.kuksa.vsscore.model.VssProperty import org.eclipse.kuksa.vsscore.model.VssSpecification import kotlin.reflect.KClass +data class VssVehicle( + val driver: VssDriver = VssDriver(), + val passenger: VssPassenger = VssPassenger(), + val body: VssBody = VssBody(), + override val uuid: String = "Vehicle", + override val vssPath: String = "Vehicle", + override val description: String = "High-level vehicle data.", + override val type: String = "branch", + override val comment: String = "", +) : VssSpecification { + override val children: Set + get() = setOf(driver, passenger, body) +} + +data class VssBody( + override val uuid: String = "Body", + override val vssPath: String = "Vehicle.Body", + override val description: String = "All body components.", + override val type: String = "branch", + override val comment: String = "", +) : VssSpecification { + override val parentClass: KClass<*> + get() = VssVehicle::class +} + data class VssDriver( val heartRate: VssHeartRate = VssHeartRate(), + val isEyesOnRoad: VssIsEyesOnRoad = VssIsEyesOnRoad(), override val uuid: String = "Driver", override val vssPath: String = "Vehicle.Driver", override val description: String = "Driver data.", @@ -34,17 +60,57 @@ data class VssDriver( override val comment: String = "", ) : VssSpecification { override val children: Set - get() = setOf(heartRate) + get() = setOf(heartRate, isEyesOnRoad) + override val parentClass: KClass<*> + get() = VssVehicle::class + + data class VssHeartRate( + override val uuid: String = "Driver HeartRate", + override val vssPath: String = "Vehicle.Driver.HeartRate", + override val description: String = "Heart rate of the driver.", + override val type: String = "sensor", + override val comment: String = "", + override val value: Int = 100, + ) : VssProperty { + override val parentClass: KClass<*> + get() = VssDriver::class + } + + data class VssIsEyesOnRoad( + override val uuid: String = "Driver IsEyesOnRoad", + override val vssPath: String = "Vehicle.Driver.IsEyesOnRoad", + override val description: String = "Has driver the eyes on road or not?", + override val type: String = "sensor", + override val comment: String = "", + override val value: Boolean = true, + ) : VssProperty { + override val parentClass: KClass<*> + get() = VssDriver::class + } } -data class VssHeartRate( - override val uuid: String = "HeartRate", - override val vssPath: String = "Vehicle.Driver.HeartRate", - override val description: String = "Heart rate of the driver.", - override val type: String = "sensor", +data class VssPassenger( + val heartRate: VssHeartRate = VssHeartRate(), + override val uuid: String = "Passenger", + override val vssPath: String = "Vehicle.Passenger", + override val description: String = "Passenger data", + override val type: String = "branch", override val comment: String = "", - override val value: Int = 100, -) : VssProperty { - override val parentClass: KClass<*>? - get() = VssDriver::class +) : VssSpecification { + override val children: Set + get() = setOf(heartRate) + override val parentClass: KClass<*> + get() = VssVehicle::class + + data class VssHeartRate( + override val uuid: String = "Passenger HeartRate", + override val vssPath: String = "Vehicle.Passenger.HeartRate", + override val description: String = "Heart rate of the Passenger.", + override val type: String = "sensor", + override val comment: String = "", + override val value: Int = 80, + ) : VssProperty { + override val parentClass: KClass<*> + get() = VssPassenger::class + } } diff --git a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssSpecification/VssSpecificationCopyTest.kt b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssSpecification/VssSpecificationCopyTest.kt index 91ed1bc6..f5da7143 100644 --- a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssSpecification/VssSpecificationCopyTest.kt +++ b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssSpecification/VssSpecificationCopyTest.kt @@ -25,27 +25,72 @@ import io.kotest.matchers.shouldBe import io.kotest.matchers.string.shouldStartWith import org.eclipse.kuksa.extension.copy import org.eclipse.kuksa.extension.deepCopy +import org.eclipse.kuksa.extension.not +import org.eclipse.kuksa.extension.plus import org.eclipse.kuksa.proto.v1.Types import org.eclipse.kuksa.proto.v1.Types.BoolArray import org.eclipse.kuksa.test.kotest.Unit +import org.eclipse.kuksa.vsscore.model.VssProperty class VssSpecificationCopyTest : BehaviorSpec({ tags(Unit) given("A specification") { - val driver = VssDriver() - val heartRate = driver.heartRate + val vehicle = VssVehicle() + var driverHeartRate: VssProperty = vehicle.driver.heartRate and("a changed heritage line") { val newValue = 70 - val updatedHeartRate = VssHeartRate(value = newValue) - val changedHeritageLine = listOf(updatedHeartRate) + val updatedHeartRate = VssDriver.VssHeartRate(value = newValue) `when`("a deep copy is done with a changed heritage line") { - val deepCopiedSpecification = driver.deepCopy(changedHeritageLine) + val deepCopiedSpecification = vehicle.deepCopy(0, updatedHeartRate) then("it should return the new children as a copy") { - val heartRateValue = deepCopiedSpecification.heartRate.value + val heartRateValue = deepCopiedSpecification.driver.heartRate.value + + heartRateValue shouldBe newValue + } + } + } + + `when`("a value copy is done via the not operator") { + val invertedSpecification = !vehicle.driver.isEyesOnRoad + + then("it should return a copy with the inverted value") { + invertedSpecification.value shouldBe false + } + } + + and("a changed value") { + val newValue = 40 + + `when`("a deep copy is done via the plus operator") { + val newHeartRate = driverHeartRate + newValue + val copiedSpecification = vehicle + newHeartRate + + then("it should return a copy with the updated value") { + val heartRateValue = copiedSpecification.driver.heartRate.value + + heartRateValue shouldBe newValue + } + } + + `when`("a value copy is done") { + val copiedSpecification = driverHeartRate + newValue + + then("it should return a copy with the updated value") { + val heartRateValue = copiedSpecification.value + + heartRateValue shouldBe newValue + } + } + + `when`("a value copy is done via assign") { + driverHeartRate += newValue + + then("it should return a copy with the updated value") { + val heartRateValue = driverHeartRate.value heartRateValue shouldBe newValue } @@ -57,7 +102,7 @@ class VssSpecificationCopyTest : BehaviorSpec({ val datapoint = Types.Datapoint.newBuilder().setInt32(newValue).build() `when`("a copy is done") { - val copiedSpecification = heartRate.copy(datapoint) + val copiedSpecification = driverHeartRate.copy(datapoint) then("it should return a copy with the updated value") { val heartRateValue = copiedSpecification.value @@ -67,10 +112,10 @@ class VssSpecificationCopyTest : BehaviorSpec({ } and("a vssPath") { - val vssPath = heartRate.vssPath + val vssPath = driverHeartRate.vssPath `when`("a copy is done") { - val copiedSpecification = heartRate.copy(vssPath, datapoint) + val copiedSpecification = driverHeartRate.copy(vssPath, datapoint) then("it should return a copy with the updated value") { val heartRateValue = copiedSpecification.value @@ -87,7 +132,7 @@ class VssSpecificationCopyTest : BehaviorSpec({ `when`("a copy is done") { val exception = shouldThrow { - heartRate.copy(datapoint) + driverHeartRate.copy(datapoint) } then("it should throw an IllegalArgumentException") { diff --git a/vss-core/src/main/java/org/eclipse/kuksa/vsscore/model/VssSpecification.kt b/vss-core/src/main/java/org/eclipse/kuksa/vsscore/model/VssSpecification.kt index 98f6c0c8..5e41c252 100644 --- a/vss-core/src/main/java/org/eclipse/kuksa/vsscore/model/VssSpecification.kt +++ b/vss-core/src/main/java/org/eclipse/kuksa/vsscore/model/VssSpecification.kt @@ -159,12 +159,26 @@ private val classNamePrefix: String /** * Creates an inheritance line to the given [heir]. Similar to [vssPathHeritageLine] but the other way around. It * returns a [Collection] of the full heritage line in the form of [VssSpecification]. - */ -fun VssSpecification.findHeritageLine(heir: VssSpecification): Collection { + * + * ### Hint + * The given heir is only used to find the heir inside the [VssSpecification]. It may differ from the one which is + * returned. If you want the heritage replaced by the given [heir] parameter then use the [isReplacingHeir] parameter. + */ +fun VssSpecification.findHeritageLine( + heir: VssSpecification, + isReplacingHeir: Boolean = false, +): Collection { val specificationKeys = heir.vssPathHeritageLine - return heritage.filter { child -> + val heritageLine = heritage.filter { child -> specificationKeys.contains(child.vssPath) + }.toMutableList() + + if (isReplacingHeir && heritageLine.isNotEmpty()) { + heritageLine.removeLast() + heritageLine.add(heir) } + + return heritageLine } /**