Property value should be updated at client side, and based on confirm…

…ation. It is possible that the responder replies non-OK message, which means updating the value is failed. In that case we should not update the value. So it should work like a transaction system - we hold pending updates to the property, and wait for the replies.
atsushieno · Mar 4, 2024 · d3e97ad · d3e97ad
1 parent 898afc8
commit d3e97ad
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 22 deletions.
diff --git a/ktmidi-ci/api/android/ktmidi-ci.api b/ktmidi-ci/api/android/ktmidi-ci.api
@@ -131,7 +131,7 @@ public final class dev/atsushieno/ktmidi/ci/ClientObservablePropertyList : dev/a
 	public fun getInternalCatalogUpdated ()Ljava/util/List;
 	public fun getMetadataList ()Ljava/util/List;
 	public final fun updateValue (Ldev/atsushieno/ktmidi/ci/Message$SubscribeProperty;)Ljava/lang/String;
-	public final fun updateValue (Ljava/lang/String;Ldev/atsushieno/ktmidi/ci/Message$GetPropertyDataReply;)V
+	public final fun updateValue (Ljava/lang/String;Ljava/util/List;Ljava/util/List;)V
 }
 
 public final class dev/atsushieno/ktmidi/ci/ClientSubscription {
@@ -1097,8 +1097,8 @@ public final class dev/atsushieno/ktmidi/ci/PropertyClientFacade {
 	public final fun sendGetPropertyData (Ldev/atsushieno/ktmidi/ci/Message$GetPropertyData;)V
 	public final fun sendGetPropertyData (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/Integer;)V
 	public static synthetic fun sendGetPropertyData$default (Ldev/atsushieno/ktmidi/ci/PropertyClientFacade;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/Integer;ILjava/lang/Object;)V
+	public final fun sendSetPropertyData (Ldev/atsushieno/ktmidi/ci/Message$SetPropertyData;)V
 	public final fun sendSetPropertyData (Ljava/lang/String;Ljava/util/List;Ljava/lang/String;Z)V
-	public final fun sendSetPropertyData (Ljava/util/List;Ljava/util/List;)V
 	public static synthetic fun sendSetPropertyData$default (Ldev/atsushieno/ktmidi/ci/PropertyClientFacade;Ljava/lang/String;Ljava/util/List;Ljava/lang/String;ZILjava/lang/Object;)V
 	public final fun sendSubscribeProperty (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
 	public static synthetic fun sendSubscribeProperty$default (Ldev/atsushieno/ktmidi/ci/PropertyClientFacade;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)V

diff --git a/ktmidi-ci/api/jvm/ktmidi-ci.api b/ktmidi-ci/api/jvm/ktmidi-ci.api
@@ -131,7 +131,7 @@ public final class dev/atsushieno/ktmidi/ci/ClientObservablePropertyList : dev/a
 	public fun getInternalCatalogUpdated ()Ljava/util/List;
 	public fun getMetadataList ()Ljava/util/List;
 	public final fun updateValue (Ldev/atsushieno/ktmidi/ci/Message$SubscribeProperty;)Ljava/lang/String;
-	public final fun updateValue (Ljava/lang/String;Ldev/atsushieno/ktmidi/ci/Message$GetPropertyDataReply;)V
+	public final fun updateValue (Ljava/lang/String;Ljava/util/List;Ljava/util/List;)V
 }
 
 public final class dev/atsushieno/ktmidi/ci/ClientSubscription {
@@ -1097,8 +1097,8 @@ public final class dev/atsushieno/ktmidi/ci/PropertyClientFacade {
 	public final fun sendGetPropertyData (Ldev/atsushieno/ktmidi/ci/Message$GetPropertyData;)V
 	public final fun sendGetPropertyData (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/Integer;)V
 	public static synthetic fun sendGetPropertyData$default (Ldev/atsushieno/ktmidi/ci/PropertyClientFacade;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/Integer;ILjava/lang/Object;)V
+	public final fun sendSetPropertyData (Ldev/atsushieno/ktmidi/ci/Message$SetPropertyData;)V
 	public final fun sendSetPropertyData (Ljava/lang/String;Ljava/util/List;Ljava/lang/String;Z)V
-	public final fun sendSetPropertyData (Ljava/util/List;Ljava/util/List;)V
 	public static synthetic fun sendSetPropertyData$default (Ldev/atsushieno/ktmidi/ci/PropertyClientFacade;Ljava/lang/String;Ljava/util/List;Ljava/lang/String;ZILjava/lang/Object;)V
 	public final fun sendSubscribeProperty (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
 	public static synthetic fun sendSubscribeProperty$default (Ldev/atsushieno/ktmidi/ci/PropertyClientFacade;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)V

diff --git a/ktmidi-ci/src/commonMain/kotlin/dev/atsushieno/ktmidi/ci/Messenger.kt b/ktmidi-ci/src/commonMain/kotlin/dev/atsushieno/ktmidi/ci/Messenger.kt
@@ -373,7 +373,7 @@ class Messenger(
 
     var processSetDataReply: (msg: Message.SetPropertyDataReply) -> Unit = { msg ->
         messageReceived.forEach { it(msg) }
-        // nothing to delegate further
+        onClient(msg, CISubId2.PROPERTY_SET_DATA_REPLY) { propertyClient.processSetDataReply(msg) }
     }
 
     var processSubscribeProperty: (msg: Message.SubscribeProperty) -> Unit = { msg ->

diff --git a/ktmidi-ci/src/commonMain/kotlin/dev/atsushieno/ktmidi/ci/ObservablePropertyList.kt b/ktmidi-ci/src/commonMain/kotlin/dev/atsushieno/ktmidi/ci/ObservablePropertyList.kt
@@ -80,10 +80,11 @@ class ClientObservablePropertyList(private val logger: Logger, private val prope
         valueUpdated.forEach { it(propertyValue) }
     }
 
-    fun updateValue(propertyId: String, reply: Message.GetPropertyDataReply) {
+    // The `header` and `body` can be either from GetPropertyDataReply or SetPropertyData
+    fun updateValue(propertyId: String, header: List<Byte>, body: List<Byte>) {
         // FIXME: cosmetic but unnecessary Common Rules for PE exposure
-        val mediaType = propertyClient.getHeaderFieldString(reply.header, PropertyCommonHeaderKeys.MEDIA_TYPE) ?: CommonRulesKnownMimeTypes.APPLICATION_JSON
-        val decodedBody = propertyClient.decodeBody(reply.header, reply.body)
+        val mediaType = propertyClient.getHeaderFieldString(header, PropertyCommonHeaderKeys.MEDIA_TYPE) ?: CommonRulesKnownMimeTypes.APPLICATION_JSON
+        val decodedBody = propertyClient.decodeBody(header, body)
         // there is no partial updates in Reply to Get Property Data
         updateValue(propertyId, false, mediaType, decodedBody)
     }

diff --git a/ktmidi-ci/src/commonMain/kotlin/dev/atsushieno/ktmidi/ci/PropertyClientFacade.kt b/ktmidi-ci/src/commonMain/kotlin/dev/atsushieno/ktmidi/ci/PropertyClientFacade.kt
@@ -14,7 +14,7 @@ class PropertyClientFacade(private val device: MidiCIDevice, private val conn: C
 
     val properties = ClientObservablePropertyList(logger, propertyRules)
 
-    private val openRequests = mutableListOf<Message.GetPropertyData>()
+    private val openRequests = mutableListOf<Message.PropertyMessage>()
     val subscriptions = mutableListOf<ClientSubscription>()
     val subscriptionUpdated = mutableListOf<(sub: ClientSubscription)->Unit>()
 
@@ -146,16 +146,22 @@ class PropertyClientFacade(private val device: MidiCIDevice, private val conn: C
             PropertyCommonHeaderKeys.MUTUAL_ENCODING to encoding,
             PropertyCommonHeaderKeys.SET_PARTIAL to isPartial))
         val encodedBody = propertyRules.encodeBody(data, encoding)
-        sendSetPropertyData(header, encodedBody)
-    }
-    // unlike the other overload, it is not specific to Common Rules for PE
-    fun sendSetPropertyData(header: List<Byte>, body: List<Byte>) =
-        messenger.send(
+        val msg =
             Message.SetPropertyData(
                 Message.Common(muid, targetMUID, MidiCIConstants.ADDRESS_FUNCTION_BLOCK, device.config.group),
-                messenger.requestIdSerial++, header, body
+                messenger.requestIdSerial++, header, encodedBody
             )
-        )
+        sendSetPropertyData(msg)
+    }
+
+    // unlike the other overload, it is not specific to Common Rules for PE
+    fun sendSetPropertyData(msg: Message.SetPropertyData) {
+        // We need to update our local property value, but we should confirm that
+        // the operation was successful by verifying Reply To Set Data message status.
+        // To ensure that, we store `msg` as a pending request.
+        openRequests.add(msg)
+        messenger.send(msg)
+    }
 
     fun sendSubscribeProperty(resource: String, mutualEncoding: String? = null, subscriptionId: String? = null) {
         val header = propertyRules.createSubscriptionHeader(resource, mapOf(
@@ -200,7 +206,20 @@ class PropertyClientFacade(private val device: MidiCIDevice, private val conn: C
             ?: return
         if (status == PropertyExchangeStatus.OK) {
             val propertyId = propertyRules.getPropertyIdForHeader(req.header)
-            properties.updateValue(propertyId, msg)
+            properties.updateValue(propertyId, msg.header, msg.body)
+            propertyRules.propertyValueUpdated(propertyId, msg.body)
+        }
+    }
+
+    internal fun processSetDataReply(msg: Message.SetPropertyDataReply) {
+        val req = openRequests.firstOrNull { it.requestId == msg.requestId }
+            ?: return
+        openRequests.remove(req)
+        val status = propertyRules.getHeaderFieldInteger(msg.header, PropertyCommonHeaderKeys.STATUS)
+            ?: return
+        if (status == PropertyExchangeStatus.OK) {
+            val propertyId = propertyRules.getPropertyIdForHeader(req.header)
+            properties.updateValue(propertyId, req.header, req.body)
             propertyRules.propertyValueUpdated(propertyId, msg.body)
         }
     }

diff --git a/.../ci/ProfileConfigurationHostFacadeTest.kt → ...tsushieno/ktmidi/ci/ProfileFacadesTest.kt b/.../ci/ProfileConfigurationHostFacadeTest.kt → ...tsushieno/ktmidi/ci/ProfileFacadesTest.kt
@@ -2,7 +2,7 @@ package dev.atsushieno.ktmidi.ci
 
 import kotlin.test.*
 
-class ProfileConfigurationHostFacadeTest {
+class ProfileFacadesTest {
     @Test
     fun configureProfiles() {
         val mediator = TestCIMediator()

diff --git a/...midi/ci/PropertyExchangeHostFacadeTest.kt → ...sushieno/ktmidi/ci/PropertyFacadesTest.kt b/...midi/ci/PropertyExchangeHostFacadeTest.kt → ...sushieno/ktmidi/ci/PropertyFacadesTest.kt
@@ -7,7 +7,7 @@ import kotlin.test.assertContentEquals
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
 
-class PropertyExchangeHostFacadeTest {
+class PropertyFacadesTest {
     @Test
     fun propertyExchange1() {
         val mediator = TestCIMediator()
@@ -30,16 +30,18 @@ class PropertyExchangeHostFacadeTest {
         // test get property
         val conn = device1.connections[device2.muid]
         assertNotNull(conn)
-        assertEquals(1, conn.propertyClient.properties.getMetadataList()!!.size, "client MetadataList size")
+        // It should contain `DeviceInfo` and `X-01` (may increase along with other predefined resources)
+        assertEquals(2, conn.propertyClient.properties.getMetadataList()!!.size, "client MetadataList size")
 
         val client = conn.propertyClient
 
         client.sendGetPropertyData(id)
-        assertContentEquals(bytes, client.properties.getProperty(id), "getProperty")
+        assertContentEquals(bytes, client.properties.getProperty(id), "client.getProperty")
 
         // test set property
         client.sendSetPropertyData(id, bytes2)
-        assertContentEquals(bytes2, host.properties.getProperty(id), "getProperty2")
+        assertContentEquals(bytes2, host.properties.getProperty(id), "host.getProperty")
+        assertContentEquals(bytes2, client.properties.getProperty(id), "client.getProperty2")
 
         // subscribe -> update value -> notify
         client.sendSubscribeProperty(id)