0.4.3-SNAPSHOT: disallow unintentionally overriding command implementations

build.gradle.kts

@@ -10,7 +10,7 @@ plugins {
 }
 
 group = "net.sergeych"
-version = "0.4.3-SNAPSHOT"
+version = "0.4.4-SNAPSHOT"
 
 repositories {
     mavenCentral()
@@ -53,7 +53,7 @@ kotlin {
                 api("org.jetbrains.kotlinx:kotlinx-serialization-json:1.4.1")
 
                 api("org.jetbrains.kotlinx:kotlinx-datetime:0.4.0")
-                api("net.sergeych:boss-serialization-mp:0.2.4-SNAPSHOT")
+                api("net.sergeych:boss-serialization-mp:0.2.7-SNAPSHOT")
                 api("net.sergeych:unikrypto:1.2.2-SNAPSHOT")
                 api("net.sergeych:mp_stools:1.3.2-SNAPSHOT")
 

src/commonMain/kotlin/net.sergeych.parsec3/AdapterBuilder.kt

@@ -25,10 +25,13 @@ class AdapterBuilder<S : WithAdapter, H : CommandHost<S>>(
     }
 
     /**
-     * Register command implementation
+     * Register command implementation.
+     * @param ca command to implement
+     * @param overwrite allow replacing existing command implementation with a new block
+     * @param block command handler that implements the command
      */
-    fun <A, R> on(ca: CommandDescriptor<A, R>, block: suspend S.(A) -> R) {
-        api.on(ca, block)
+    fun <A, R> on(ca: CommandDescriptor<A, R>,overwrite: Boolean=false, block: suspend S.(A) -> R) {
+        api.on(ca, overwrite, block)
     }
 
     val onCancelHandlers = mutableListOf<suspend () -> Unit>()

src/commonMain/kotlin/net.sergeych.parsec3/CommandDescriptor.kt

@@ -13,8 +13,8 @@ class CommandDescriptor<A, R>(
     @Suppress("UNCHECKED_CAST")
     suspend operator fun invoke(adapter: Adapter<*>): R = adapter.invokeCommand(this,Unit as A)
 
-    operator fun <I: WithAdapter>invoke(commandHost: CommandHost<I>, block: suspend I.(A)->R) {
-        commandHost.on(this, block)
+    operator fun <I: WithAdapter>invoke(commandHost: CommandHost<I>, overwrite: Boolean = false,block: suspend I.(A)->R) {
+        commandHost.on(this, overwrite, block)
     }
 }
 

src/commonMain/kotlin/net.sergeych.parsec3/CommandHost.kt

@@ -34,10 +34,16 @@ open class CommandHost<T: WithAdapter> {
     private val handlers = mutableMapOf<String, suspend T.(ByteArray) -> ByteArray>()
 
     /**
-     * Provide implementation for a specific command in type-safe compile-time checked manner. the command
-     * should be declared with [command] invocation.
+     * Provide implementation for a specific command in a type-safe compile-time checked manner.
+     * The command should be declared with [command] invocation.
+     *
+     * Normally, existing command can't be replaced by new handlers and attempt to do so will throw
+     * [DuplicateCommandDefinition] at runtime. If it is done intentionally, set [overwrite] to
+     * true.
      */
-    fun <A, R> on(ca: CommandDescriptor<A, R>, block: suspend T.(A) -> R) {
+    fun <A, R> on(ca: CommandDescriptor<A, R>, overwrite: Boolean = false, block: suspend T.(A) -> R) {
+        if( ca.name in handlers && !overwrite )
+            throw DuplicateCommandDefinition("command already defined: ${ca.name}")
         handlers[ca.name] = {args ->
             val decodedArgs = BossDecoder.decodeFrom<A>(ca.ass, args)
             BossEncoder.encode(ca.rss, block(decodedArgs))

src/commonMain/kotlin/net.sergeych.parsec3/ParsecException.kt

@@ -16,4 +16,11 @@ class InvalidFrameException(text: String): ParsecException(ExceptionsRegistry.in
 
 class UnknownCodeException(code: String,message: String?): ParsecException(code, message)
 
-class UnknownException(message: String?): ParsecException(ExceptionsRegistry.unknownErrorCode, message)
+class UnknownException(message: String?): ParsecException(ExceptionsRegistry.unknownErrorCode, message)
+
+/**
+ * This exception is server-side, not intended to arise while processing commands, so it is not
+ * a parsec exception. It means some adapter is trying to redefine existing command, which is
+ * generally a fault.
+ */
+class DuplicateCommandDefinition(message: String) : RuntimeException(message)

src/commonTest/kotlin/parsec3/AdapterTest.kt

@@ -58,6 +58,7 @@ internal class AdapterTest {
         val foo by command<String, String>()
         val ex by command<String,Unit>()
         val ex2 by command<String,Unit>()
+        val nullableTest by command<Int,String?>()
     }
     class ApiS2<T: WithAdapter> : CommandHost<T>() {
         // create command `foo` that takes a string argument and
@@ -88,6 +89,9 @@ internal class AdapterTest {
             on(ApiS1.ex2) {
                 throw IllegalArgumentException()
             }
+            on(ApiS1.nullableTest) {
+                if( it == 1 ) "one" else null
+            }
             onCancel {
                 b1Cancelled = true
             }
@@ -120,6 +124,9 @@ internal class AdapterTest {
         assertEquals("%% loop-42foo %%", a1.invokeCommand(ApiS2<WithAdapter>().loopCall, "123"))
         assertEquals("32142foo", a2.invokeCommand(ApiS1.foo, "321"))
 
+        assertEquals("one", a2.invokeCommand(ApiS1.nullableTest,1))
+        assertEquals(null, a2.invokeCommand(ApiS1.nullableTest,2))
+
         assertEquals("---42foo", ApiS1.foo.invoke(a2, "---"))
         val x = assertThrows<IllegalArgumentException> { ApiS1.ex.invoke(a2, "foobar") }
         assertEquals("foobar", x.message)
@@ -133,5 +140,18 @@ internal class AdapterTest {
         assertTrue { b1Cancelled }
         assertTrue { b2Cancelled }
     }
-
+    @Test
+    fun builderDupeErrorTest() = runTest {
+        assertThrows<DuplicateCommandDefinition> {
+            AdapterBuilder(ApiS1) {
+                newSession { TestSession("42") }
+                on(api.foo) {
+                    it + buzz + "foo"
+                }
+                on(api.foo) {
+                    it + buzz + "foo"
+                }
+            }
+        }
+    }
 }