Add structured send methods

CHANGELOG

@@ -3,6 +3,12 @@ vNEXT (in development)
  * Batch start and end events are no longer included in BatchReceived events
  * Batches now expose complete metadata from their start event
  * Added support for labelled-replies capability and label message tags
+ * Added new methods for sending raw lines to the IRC server
+   * send(tagMap(...), command, arguments) replaces send(line)
+   * send(command, arguments) is available if no tags are to be sent
+   * the line is built automatically (with spaces/' :' added appropriately)
+   * send(line) is deprecated and will be removed after v1.0.0
+ * (Internal) Added annotation to track removal of deprecated methods
 
 v0.9.0
 

src/main/kotlin/com/dmdirc/ktirc/IrcClient.kt

@@ -4,6 +4,7 @@ import com.dmdirc.ktirc.events.IrcEvent
 import com.dmdirc.ktirc.io.CaseMapping
 import com.dmdirc.ktirc.messages.sendJoin
 import com.dmdirc.ktirc.model.*
+import com.dmdirc.ktirc.util.RemoveIn
 
 /**
  * Primary interface for interacting with KtIrc.
@@ -54,8 +55,33 @@ interface IrcClient {
      *
      * @param message The line to be sent to the IRC server.
      */
+    @Deprecated("Use structured send instead", ReplaceWith("send(command, arguments)"))
+    @RemoveIn("2.0.0")
     fun send(message: String)
 
+    /**
+     * Sends the given command to the IRC server.
+     *
+     * This should only be needed to send raw/custom commands; standard messages can be sent using the
+     * extension methods in [com.dmdirc.ktirc.messages] such as [sendJoin].
+     *
+     * @param tags The IRCv3 tags to prefix the message with, if any.
+     * @param command The command to be sent
+     * @param arguments The arguments to the command.
+     */
+    fun send(tags: Map<MessageTag, String>, command: String, vararg arguments: String)
+
+    /**
+     * Sends the given command to the IRC server.
+     *
+     * This should only be needed to send raw/custom commands; standard messages can be sent using the
+     * extension methods in [com.dmdirc.ktirc.messages] such as [sendJoin].
+     *
+     * @param command The command to be sent
+     * @param arguments The arguments to the command.
+     */
+    fun send(command: String, vararg arguments: String) = send(emptyMap(), command, *arguments)
+
     /**
      * Registers a new handler for all events on this connection.
      *

src/main/kotlin/com/dmdirc/ktirc/IrcClientImpl.kt

@@ -40,6 +40,7 @@ internal class IrcClientImpl(private val config: IrcClientConfig) : IrcClient, C
     override val userState = UserState { caseMapping }
 
     private val messageHandler = MessageHandler(messageProcessors, eventMutators, eventHandlers)
+    private val messageBuilder = MessageBuilder()
 
     private val parser = MessageParser()
     private var socket: LineBufferedSocket? = null
@@ -50,20 +51,20 @@ internal class IrcClientImpl(private val config: IrcClientConfig) : IrcClient, C
         socket?.sendChannel?.offer(message.toByteArray()) ?: log.warning { "No send channel for message: $message" }
     }
 
+    override fun send(tags: Map<MessageTag, String>, command: String, vararg arguments: String) {
+        socket?.sendChannel?.offer(messageBuilder.build(tags, command, arguments))
+                ?: log.warning { "No send channel for command: $command" }
+    }
+
     // TODO: This will become sendAsync and return a Deferred<IrcEvent>
-    // TODO: Refactor so that send takes a map of tags and arguments; build the string separately
-    internal fun sendWithLabel(message: String) {
-        val messageToSend = if (Capability.LabeledResponse in serverState.capabilities.enabledCapabilities) {
-            val label = generateLabel(this)
-            "@draft/label=$label" + if (message.startsWith('@')) {
-                ";${message.substring(1)}"
-            } else {
-                " $message"
-            }
+    internal fun sendWithLabel(tags: Map<MessageTag, String>, command: String, vararg arguments: String) {
+        val tagseToSend = if (Capability.LabeledResponse in serverState.capabilities.enabledCapabilities) {
+            tags + (MessageTag.Label to generateLabel(this))
         } else {
-            message
+            tags
         }
-        socket?.sendChannel?.offer(messageToSend.toByteArray()) ?: log.warning { "No send channel for message: $message" }
+        socket?.sendChannel?.offer(messageBuilder.build(tagseToSend, command, arguments))
+                ?: log.warning { "No send channel for command: $command" }
     }
 
     override fun connect() {

src/main/kotlin/com/dmdirc/ktirc/events/Events.kt

@@ -4,6 +4,7 @@ import com.dmdirc.ktirc.model.Capability
 import com.dmdirc.ktirc.model.ConnectionError
 import com.dmdirc.ktirc.model.ServerFeatureMap
 import com.dmdirc.ktirc.model.User
+import com.dmdirc.ktirc.util.RemoveIn
 import java.time.LocalDateTime
 
 /**
@@ -24,7 +25,8 @@ data class EventMetadata(
 sealed class IrcEvent(val metadata: EventMetadata) {
 
     /** The time at which the event occurred. */
-    @Deprecated("Moved to metadata; to be removed post-1.0.0", replaceWith = ReplaceWith("metadata.time"))
+    @Deprecated("Moved to metadata", replaceWith = ReplaceWith("metadata.time"))
+    @RemoveIn("2.0.0")
     val time: LocalDateTime
         get() = metadata.time
 
@@ -92,7 +94,8 @@ class ChannelTopicChanged(metadata: EventMetadata, val user: User, val channel:
 class MessageReceived(metadata: EventMetadata, val user: User, val target: String, val message: String) : IrcEvent(metadata) {
 
     /** The message ID of this message. */
-    @Deprecated("Moved to metadata; to be removed post-1.0.0", replaceWith = ReplaceWith("metadata.messageId"))
+    @Deprecated("Moved to metadata", replaceWith = ReplaceWith("metadata.messageId"))
+    @RemoveIn("2.0.0")
     val messageId: String?
         get() = metadata.messageId
 
@@ -109,7 +112,8 @@ class NoticeReceived(metadata: EventMetadata, val user: User, val target: String
 class ActionReceived(metadata: EventMetadata, val user: User, val target: String, val action: String) : IrcEvent(metadata) {
 
     /** The message ID of this action. */
-    @Deprecated("Moved to metadata; to be removed post-1.0.0", replaceWith = ReplaceWith("metadata.messageId"))
+    @Deprecated("Moved to metadata", replaceWith = ReplaceWith("metadata.messageId"))
+    @RemoveIn("2.0.0")
     val messageId: String?
         get() = metadata.messageId
 

src/main/kotlin/com/dmdirc/ktirc/messages/MessageBuilder.kt

@@ -0,0 +1,41 @@
+package com.dmdirc.ktirc.messages
+
+import com.dmdirc.ktirc.model.MessageTag
+
+internal class MessageBuilder {
+
+    fun build(tags: Map<MessageTag, String>, command: String, arguments: Array<out String>) =
+            // TODO: Check line length
+            buildString {
+                append(tags.toPrefix())
+                append(command)
+                append(arguments.toSuffix())
+            }.toByteArray()
+
+    private fun Map<MessageTag, String>.toPrefix() = when {
+        isEmpty() -> ""
+        // TODO: Check if the server actually understands tags here
+        // TODO: Check maximum length of tags
+        else ->
+            map { "${it.key.name}=${it.value.escapeTagValue()}" }
+                    .joinToString(separator = ";", prefix = "@", postfix = " ")
+    }
+
+    private fun Array<out String>.toSuffix() = when (size) {
+        0 -> ""
+        1 -> this[0].asLastParam()
+        else -> dropLast(1).joinToString(separator = " ", prefix = " ") + last().asLastParam()
+    }
+
+    private fun String.asLastParam() = when {
+        contains(' ') || startsWith(':') -> " :$this"
+        else -> " $this"
+    }
+
+    private fun String.escapeTagValue() = replace("\\", "\\\\")
+            .replace("\n", "\\n")
+            .replace("\r", "\\r")
+            .replace(";", "\\:")
+            .replace(" ", "\\s")
+
+}

src/main/kotlin/com/dmdirc/ktirc/messages/MessageBuilders.kt

@@ -4,28 +4,28 @@ import com.dmdirc.ktirc.IrcClient
 import com.dmdirc.ktirc.model.MessageTag
 
 /** Sends a message to ask the server to list capabilities. */
-internal fun IrcClient.sendCapabilityList() = send("CAP LS 302")
+internal fun IrcClient.sendCapabilityList() = send("CAP", "LS", "302")
 
 /** Sends a message indicating the end of capability negotiation. */
-internal fun IrcClient.sendCapabilityEnd() = send("CAP END")
+internal fun IrcClient.sendCapabilityEnd() = send("CAP", "END")
 
 /** Sends a message requesting the specified caps are enabled. */
-internal fun IrcClient.sendCapabilityRequest(capabilities: List<String>) = send("CAP REQ :${capabilities.joinToString(" ")}")
+internal fun IrcClient.sendCapabilityRequest(capabilities: List<String>) = send("CAP", "REQ", capabilities.joinToString(" "))
 
 /** Sends a request to join the given channel. */
-fun IrcClient.sendJoin(channel: String) = send("JOIN :$channel")
+fun IrcClient.sendJoin(channel: String) = send("JOIN", channel)
 
 /** Sends a request to see the modes of a given target. */
-fun IrcClient.sendModeRequest(target: String) = send("MODE :$target")
+fun IrcClient.sendModeRequest(target: String) = send("MODE", target)
 
 /** Sends a request to change to the given nickname. */
-fun IrcClient.sendNickChange(nick: String) = send("NICK :$nick")
+fun IrcClient.sendNickChange(nick: String) = send("NICK", nick)
 
 /** Sends the connection password to the server. */
-internal fun IrcClient.sendPassword(password: String) = send("PASS :$password")
+internal fun IrcClient.sendPassword(password: String) = send("PASS", password)
 
 /** Sends a response to a PING event. */
-internal fun IrcClient.sendPong(nonce: ByteArray) = send("PONG :${String(nonce)}")
+internal fun IrcClient.sendPong(nonce: ByteArray) = send("PONG", String(nonce))
 
 /** Sends a CTCP message of the specified [type] and with optional [data] to [target] (a user or a channel). */
 fun IrcClient.sendCtcp(target: String, type: String, data: String? = null) =
@@ -36,7 +36,11 @@ fun IrcClient.sendAction(target: String, action: String) = sendCtcp(target, "ACT
 
 /** Sends a private message to a user or channel. */
 fun IrcClient.sendMessage(target: String, message: String, inReplyTo: String? = null) =
-        sendWithTags(mapOf(MessageTag.Reply to inReplyTo), "PRIVMSG $target :$message")
+        send(
+                inReplyTo?.let { tagMap(MessageTag.Reply to inReplyTo) } ?: emptyMap(),
+                "PRIVMSG",
+                target,
+                message)
 
 /**
  * Sends a tag-only message.
@@ -44,31 +48,16 @@ fun IrcClient.sendMessage(target: String, message: String, inReplyTo: String? =
  * If [inReplyTo] is specified then the [MessageTag.Reply] tag will be automatically added.
  */
 fun IrcClient.sendTagMessage(target: String, tags: Map<MessageTag, String>, inReplyTo: String? = null) {
-    sendWithTags(inReplyTo?.let { tags + (MessageTag.Reply to inReplyTo) } ?: tags, "TAGMSG $target")
+    send(inReplyTo?.let { tags + (MessageTag.Reply to inReplyTo) } ?: tags, "TAGMSG", target)
 }
 
 /** Sends a message to register a user with the server. */
-internal fun IrcClient.sendUser(userName: String, realName: String) = send("USER $userName 0 * :$realName")
+internal fun IrcClient.sendUser(userName: String, realName: String) = send("USER", userName, "0", "*", realName)
 
 /** Starts an authentication request. */
-internal fun IrcClient.sendAuthenticationMessage(data: String = "+") = send("AUTHENTICATE $data")
+internal fun IrcClient.sendAuthenticationMessage(data: String = "+") = send("AUTHENTICATE", data)
 
 /**
- * Sends a message prefixed with some IRCv3 tags.
- *
- * For convenience, if the value of a tag is `null`, the tag will be omitted. If no tags are present the
- * message is sent directly with no prefix.
+ * Utility method for creating a map of tags to avoid type inference problems.
  */
-internal fun IrcClient.sendWithTags(tags: Map<MessageTag, String?>, message: String) = tags
-        .filterValues { it != null }
-        .map { (key, value) -> "${key.name}=${value?.escapeTagValue()}" }
-        .joinToString(";")
-        .let {
-            if (it.isEmpty()) send(message) else send("@$it $message")
-        }
-
-internal fun String.escapeTagValue() = replace("\\", "\\\\")
-        .replace("\n", "\\n")
-        .replace("\r", "\\r")
-        .replace(";", "\\:")
-        .replace(" ", "\\s")
+fun tagMap(vararg tags: Pair<MessageTag, String>) = mapOf(*tags)

src/main/kotlin/com/dmdirc/ktirc/util/RemoveIn.kt

@@ -0,0 +1,10 @@
+package com.dmdirc.ktirc.util
+
+/**
+ * Documents when a deprecated feature will be removed.
+ */
+@MustBeDocumented
+@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY, AnnotationTarget.ANNOTATION_CLASS,
+        AnnotationTarget.CONSTRUCTOR, AnnotationTarget.PROPERTY_SETTER, AnnotationTarget.PROPERTY_GETTER,
+        AnnotationTarget.TYPEALIAS)
+annotation class RemoveIn(val version: String)

src/test/kotlin/com/dmdirc/ktirc/IrcClientImplTest.kt

@@ -3,6 +3,7 @@ package com.dmdirc.ktirc
 import com.dmdirc.ktirc.events.*
 import com.dmdirc.ktirc.io.CaseMapping
 import com.dmdirc.ktirc.io.LineBufferedSocket
+import com.dmdirc.ktirc.messages.tagMap
 import com.dmdirc.ktirc.model.*
 import com.dmdirc.ktirc.util.currentTimeProvider
 import com.dmdirc.ktirc.util.generateLabel
@@ -122,7 +123,7 @@ internal class IrcClientImplTest {
         client.connect()
 
         assertEquals("CAP LS 302", String(sendLineChannel.receive()))
-        assertEquals("NICK :$NICK", String(sendLineChannel.receive()))
+        assertEquals("NICK $NICK", String(sendLineChannel.receive()))
         assertEquals("USER $USER_NAME 0 * :$REAL_NAME", String(sendLineChannel.receive()))
     }
 
@@ -137,7 +138,7 @@ internal class IrcClientImplTest {
         client.connect()
 
         assertEquals("CAP LS 302", String(sendLineChannel.receive()))
-        assertEquals("PASS :$PASSWORD", String(sendLineChannel.receive()))
+        assertEquals("PASS $PASSWORD", String(sendLineChannel.receive()))
     }
 
     @Test
@@ -199,13 +200,35 @@ internal class IrcClientImplTest {
         assertLineReceived("testing 123")
     }
 
+    @Test
+    fun `sends structured text to socket`() = runBlocking {
+        val client = IrcClientImpl(normalConfig)
+        client.socketFactory = mockSocketFactory
+        client.connect()
+
+        client.send("testing", "123", "456")
+
+        assertLineReceived("testing 123 456")
+    }
+
+    @Test
+    fun `sends structured text to socket with tags`() = runBlocking {
+        val client = IrcClientImpl(normalConfig)
+        client.socketFactory = mockSocketFactory
+        client.connect()
+
+        client.send(tagMap(MessageTag.AccountName to "acidB"), "testing", "123", "456")
+
+        assertLineReceived("@account=acidB testing 123 456")
+    }
+
     @Test
     fun `sends text to socket without label if cap is missing`() = runBlocking {
         val client = IrcClientImpl(normalConfig)
         client.socketFactory = mockSocketFactory
         client.connect()
 
-        client.sendWithLabel("testing 123")
+        client.sendWithLabel(tagMap(), "testing", "123")
 
         assertLineReceived("testing 123")
     }
@@ -218,7 +241,7 @@ internal class IrcClientImplTest {
         client.serverState.capabilities.enabledCapabilities[Capability.LabeledResponse] = ""
         client.connect()
 
-        client.sendWithLabel("testing 123")
+        client.sendWithLabel(tagMap(), "testing", "123")
 
         assertLineReceived("@draft/label=abc123 testing 123")
     }
@@ -231,9 +254,9 @@ internal class IrcClientImplTest {
         client.serverState.capabilities.enabledCapabilities[Capability.LabeledResponse] = ""
         client.connect()
 
-        client.sendWithLabel("@+test=x testing 123")
+        client.sendWithLabel(tagMap(MessageTag.AccountName to "x"), "testing", "123")
 
-        assertLineReceived("@draft/label=abc123;+test=x testing 123")
+        assertLineReceived("@account=x;draft/label=abc123 testing 123")
     }
 
     @Test
@@ -254,7 +277,7 @@ internal class IrcClientImplTest {
         client.socketFactory = mockSocketFactory
         client.connect()
 
-        (0..100).forEach { client.send("TEST $it") }
+        (0..100).forEach { client.send("TEST", "$it") }
 
         assertEquals(100, withTimeoutOrNull(500) {
             var next = 0

src/test/kotlin/com/dmdirc/ktirc/events/EventUtilsTest.kt

@@ -3,10 +3,8 @@ package com.dmdirc.ktirc.events
 import com.dmdirc.ktirc.IrcClient
 import com.dmdirc.ktirc.TestConstants
 import com.dmdirc.ktirc.io.CaseMapping
-import com.dmdirc.ktirc.model.ModePrefixMapping
-import com.dmdirc.ktirc.model.ServerFeature
-import com.dmdirc.ktirc.model.ServerState
-import com.dmdirc.ktirc.model.User
+import com.dmdirc.ktirc.messages.tagMap
+import com.dmdirc.ktirc.model.*
 import com.nhaarman.mockitokotlin2.doReturn
 import com.nhaarman.mockitokotlin2.mock
 import com.nhaarman.mockitokotlin2.verify
@@ -72,7 +70,7 @@ internal class EventUtilsTest {
         val message = MessageReceived(EventMetadata(TestConstants.time), User("acidBurn"), "Zerocool", "Hack the planet!")
 
         ircClient.reply(message, "OK")
-        verify(ircClient).send("PRIVMSG acidBurn :OK")
+        verify(ircClient).send(tagMap(), "PRIVMSG", "acidBurn", "OK")
     }
 
     @Test
@@ -80,7 +78,7 @@ internal class EventUtilsTest {
         val message = MessageReceived(EventMetadata(TestConstants.time), User("acidBurn"), "#TheGibson", "Hack the planet!")
 
         ircClient.reply(message, "OK")
-        verify(ircClient).send("PRIVMSG #TheGibson :OK")
+        verify(ircClient).send(tagMap(), "PRIVMSG", "#TheGibson", "OK")
     }
 
     @Test
@@ -88,7 +86,7 @@ internal class EventUtilsTest {
         val message = MessageReceived(EventMetadata(TestConstants.time), User("acidBurn"), "#TheGibson", "Hack the planet!")
 
         ircClient.reply(message, "OK", prefixWithNickname = true)
-        verify(ircClient).send("PRIVMSG #TheGibson :acidBurn: OK")
+        verify(ircClient).send(tagMap(), "PRIVMSG", "#TheGibson", "acidBurn: OK")
     }
 
     @Test
@@ -97,7 +95,7 @@ internal class EventUtilsTest {
         val message = MessageReceived(EventMetadata(TestConstants.time, messageId = "abc123"), User("acidBurn"), "Zerocool", "Hack the planet!")
 
         ircClient.reply(message, "OK")
-        verify(ircClient).send("@+draft/reply=abc123 PRIVMSG acidBurn :OK")
+        verify(ircClient).send(tagMap(MessageTag.Reply to "abc123"), "PRIVMSG", "acidBurn", "OK")
     }
 
     @Test
@@ -105,7 +103,7 @@ internal class EventUtilsTest {
         val message = MessageReceived(EventMetadata(TestConstants.time, messageId = "abc123"), User("acidBurn"), "#TheGibson", "Hack the planet!")
 
         ircClient.reply(message, "OK")
-        verify(ircClient).send("@+draft/reply=abc123 PRIVMSG #TheGibson :OK")
+        verify(ircClient).send(tagMap(MessageTag.Reply to "abc123"), "PRIVMSG", "#TheGibson", "OK")
     }
 
     @Test
@@ -113,7 +111,7 @@ internal class EventUtilsTest {
         val message = MessageReceived(EventMetadata(TestConstants.time, messageId = "abc123"), User("acidBurn"), "#TheGibson", "Hack the planet!")
 
         ircClient.reply(message, "OK", prefixWithNickname = true)
-        verify(ircClient).send("@+draft/reply=abc123 PRIVMSG #TheGibson :acidBurn: OK")
+        verify(ircClient).send(tagMap(MessageTag.Reply to "abc123"), "PRIVMSG", "#TheGibson", "acidBurn: OK")
     }
 
 
@@ -123,7 +121,7 @@ internal class EventUtilsTest {
         val message = MessageReceived(EventMetadata(TestConstants.time, messageId = "msgId"), User("acidBurn"), "Zerocool", "Hack the planet!")
 
         ircClient.react(message, ":P")
-        verify(ircClient).send("@+draft/react=:P;+draft/reply=msgId TAGMSG acidBurn")
+        verify(ircClient).send(tagMap(MessageTag.React to ":P", MessageTag.Reply to "msgId"), "TAGMSG", "acidBurn")
     }
 
     @Test
@@ -131,7 +129,7 @@ internal class EventUtilsTest {
         val message = MessageReceived(EventMetadata(TestConstants.time, messageId = "msgId"), User("acidBurn"), "#TheGibson", "Hack the planet!")
 
         ircClient.react(message, ":P")
-        verify(ircClient).send("@+draft/react=:P;+draft/reply=msgId TAGMSG #TheGibson")
+        verify(ircClient).send(tagMap(MessageTag.React to ":P", MessageTag.Reply to "msgId"), "TAGMSG", "#TheGibson")
     }
 
 }

src/test/kotlin/com/dmdirc/ktirc/events/handlers/CapabilitiesHandlerTest.kt

@@ -64,14 +64,14 @@ internal class CapabilitiesHandlerTest {
 
         handler.processEvent(ircClient, ServerCapabilitiesFinished(EventMetadata(TestConstants.time)))
 
-        verify(ircClient).send(argThat { equals("CAP REQ :echo-message account-notify") || equals("CAP REQ :account-notify echo-message") })
+        verify(ircClient).send(eq("CAP"), eq("REQ"), argThat { equals("echo-message account-notify") || equals("account-notify echo-message") })
     }
 
     @Test
     fun `sends END when blank capabilities received`() {
         handler.processEvent(ircClient, ServerCapabilitiesFinished(EventMetadata(TestConstants.time)))
 
-        verify(ircClient).send("CAP END")
+        verify(ircClient).send("CAP", "END")
     }
 
     @Test
@@ -88,7 +88,7 @@ internal class CapabilitiesHandlerTest {
                 Capability.HostsInNamesReply to ""
         )))
 
-        verify(ircClient).send("CAP END")
+        verify(ircClient).send("CAP", "END")
     }
 
     @Test
@@ -99,7 +99,7 @@ internal class CapabilitiesHandlerTest {
                 Capability.HostsInNamesReply to ""
         )))
 
-        verify(ircClient).send("CAP END")
+        verify(ircClient).send("CAP", "END")
     }
 
     @Test
@@ -111,7 +111,7 @@ internal class CapabilitiesHandlerTest {
                 Capability.HostsInNamesReply to ""
         )))
 
-        verify(ircClient).send("CAP END")
+        verify(ircClient).send("CAP", "END")
     }
 
     @Test
@@ -147,7 +147,7 @@ internal class CapabilitiesHandlerTest {
                 Capability.HostsInNamesReply to ""
         )))
 
-        verify(ircClient).send("AUTHENTICATE mech1")
+        verify(ircClient).send("AUTHENTICATE", "mech1")
     }
 
     @Test
@@ -159,7 +159,7 @@ internal class CapabilitiesHandlerTest {
                 Capability.HostsInNamesReply to ""
         )))
 
-        verify(ircClient).send("AUTHENTICATE mech3")
+        verify(ircClient).send("AUTHENTICATE", "mech3")
     }
 
     @Test
@@ -201,7 +201,7 @@ internal class CapabilitiesHandlerTest {
         serverState.sasl.currentMechanism = null
         handler.processEvent(ircClient, AuthenticationMessage(EventMetadata(TestConstants.time), "+"))
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -267,7 +267,7 @@ internal class CapabilitiesHandlerTest {
     fun `sends END when SASL auth finished`() {
         handler.processEvent(ircClient, SaslFinished(EventMetadata(TestConstants.time), true))
 
-        verify(ircClient).send("CAP END")
+        verify(ircClient).send("CAP", "END")
     }
 
     @Test
@@ -299,7 +299,7 @@ internal class CapabilitiesHandlerTest {
         serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
         handler.processEvent(ircClient, SaslMechanismNotAvailableError(EventMetadata(TestConstants.time), listOf("mech1", "fake2")))
 
-        verify(ircClient).send("AUTHENTICATE mech1")
+        verify(ircClient).send("AUTHENTICATE", "mech1")
     }
 
     @Test
@@ -307,7 +307,7 @@ internal class CapabilitiesHandlerTest {
         serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
         handler.processEvent(ircClient, SaslMechanismNotAvailableError(EventMetadata(TestConstants.time), listOf("fake1", "fake2")))
 
-        verify(ircClient).send("CAP END")
+        verify(ircClient).send("CAP", "END")
     }
 
     @Test

src/test/kotlin/com/dmdirc/ktirc/events/handlers/ChannelStateHandlerTest.kt

@@ -145,7 +145,7 @@ internal class ChannelStateHandlerTest {
 
         handler.processEvent(ircClient, ChannelNamesFinished(EventMetadata(TestConstants.time), "#thegibson"))
 
-        verify(ircClient).send("MODE :#thegibson")
+        verify(ircClient).send("MODE", "#thegibson")
     }
 
     @Test
@@ -158,7 +158,7 @@ internal class ChannelStateHandlerTest {
 
         handler.processEvent(ircClient, ChannelNamesFinished(EventMetadata(TestConstants.time), "#thegibson"))
 
-        verify(ircClient, never()).send("MODE :#thegibson")
+        verify(ircClient, never()).send("MODE", "#thegibson")
     }
 
     @Test
@@ -170,7 +170,7 @@ internal class ChannelStateHandlerTest {
 
         handler.processEvent(ircClient, ChannelNamesFinished(EventMetadata(TestConstants.time), "#thegibson"))
 
-        verify(ircClient, never()).send("MODE :#thegibson")
+        verify(ircClient, never()).send("MODE", "#thegibson")
     }
 
     @Test

src/test/kotlin/com/dmdirc/ktirc/events/handlers/PingHandlerTest.kt

@@ -18,7 +18,7 @@ internal class PingHandlerTest {
     @Test
     fun `PingHandler responses to pings with a pong`() = runBlocking {
         handler.processEvent(ircClient, PingReceived(EventMetadata(TestConstants.time), "the_plague".toByteArray()))
-        verify(ircClient).send("PONG :the_plague")
+        verify(ircClient).send("PONG", "the_plague")
     }
 
 }

src/test/kotlin/com/dmdirc/ktirc/messages/MessageBuilderTest.kt

@@ -0,0 +1,62 @@
+package com.dmdirc.ktirc.messages
+
+import com.dmdirc.ktirc.model.MessageTag
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Test
+
+internal class MessageBuilderTest {
+
+    private val builder = MessageBuilder()
+
+    @Test
+    fun `builds a command on its own`() =
+            assertEquals("TEST", String(builder.build(emptyMap(), "TEST", emptyArray())))
+
+    @Test
+    fun `handles a single argument`() =
+            assertEquals("TEST foo", String(builder.build(emptyMap(), "TEST", arrayOf("foo"))))
+
+    @Test
+    fun `handles a single argument starting with a colon`() =
+            assertEquals("TEST ::foo", String(builder.build(emptyMap(), "TEST", arrayOf(":foo"))))
+
+    @Test
+    fun `handles a single argument with spaces`() =
+            assertEquals("TEST :foo bar", String(builder.build(emptyMap(), "TEST", arrayOf("foo bar"))))
+
+    @Test
+    fun `handles many arguments`() =
+            assertEquals("TEST foo bar baz", String(builder.build(emptyMap(), "TEST", arrayOf("foo", "bar", "baz"))))
+
+    @Test
+    fun `handles many arguments with spaces in the last`() =
+            assertEquals("TEST foo bar :baz quux", String(builder.build(emptyMap(), "TEST", arrayOf("foo", "bar", "baz quux"))))
+
+    @Test
+    fun `handles single tag`() =
+            assertEquals(
+                    "@draft/label=abc TEST foo bar",
+                    String(builder.build(
+                            mapOf(MessageTag.Label to "abc"),
+                            "TEST",
+                            arrayOf("foo", "bar"))))
+
+    @Test
+    fun `handles multiple tags`() =
+            assertEquals(
+                    "@draft/label=abc;account=acidB TEST foo bar",
+                    String(builder.build(
+                            mapOf(MessageTag.Label to "abc", MessageTag.AccountName to "acidB"),
+                            "TEST",
+                            arrayOf("foo", "bar"))))
+
+    @Test
+    fun `escapes tag values`() =
+            assertEquals(
+                    "@draft/label=\\\\hack\\sthe\\r\\nplanet\\: TEST foo bar",
+                    String(builder.build(
+                            mapOf(MessageTag.Label to "\\hack the\r\nplanet;"),
+                            "TEST",
+                            arrayOf("foo", "bar"))))
+
+}

src/test/kotlin/com/dmdirc/ktirc/messages/MessageBuildersTest.kt

@@ -4,7 +4,6 @@ import com.dmdirc.ktirc.IrcClient
 import com.dmdirc.ktirc.model.MessageTag
 import com.nhaarman.mockitokotlin2.mock
 import com.nhaarman.mockitokotlin2.verify
-import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Test
 
 internal class MessageBuildersTest {
@@ -14,132 +13,105 @@ internal class MessageBuildersTest {
     @Test
     fun `sendCapabilityRequest sends CAP REQ message with single argument`() {
         mockClient.sendCapabilityRequest(listOf("a"))
-        verify(mockClient).send("CAP REQ :a")
+        verify(mockClient).send("CAP", "REQ", "a")
     }
 
     @Test
     fun `sendCapabilityRequest sends CAP REQ message with multiple args`() {
         mockClient.sendCapabilityRequest(listOf("a b c"))
-        verify(mockClient).send("CAP REQ :a b c")
+        verify(mockClient).send("CAP", "REQ", "a b c")
     }
 
     @Test
     fun `sendJoin sends correct JOIN message`() {
         mockClient.sendJoin("#TheGibson")
-        verify(mockClient).send("JOIN :#TheGibson")
+        verify(mockClient).send("JOIN", "#TheGibson")
     }
 
     @Test
     fun `sendModeRequest sends correct MODE message`() {
         mockClient.sendModeRequest("#TheGibson")
-        verify(mockClient).send("MODE :#TheGibson")
+        verify(mockClient).send("MODE", "#TheGibson")
     }
 
     @Test
     fun `sendNickChange sends correct NICK message`() {
         mockClient.sendNickChange("AcidBurn")
-        verify(mockClient).send("NICK :AcidBurn")
+        verify(mockClient).send("NICK", "AcidBurn")
     }
 
     @Test
     fun `sendPassword sends correct PASS message`() {
         mockClient.sendPassword("hacktheplanet")
-        verify(mockClient).send("PASS :hacktheplanet")
+        verify(mockClient).send("PASS", "hacktheplanet")
     }
 
     @Test
     fun `sendPong sends correct PONG message`() {
         mockClient.sendPong("abcdef".toByteArray())
-        verify(mockClient).send("PONG :abcdef")
+        verify(mockClient).send("PONG", "abcdef")
     }
 
     @Test
     fun `sendMessage sends correct PRIVMSG message`() {
         mockClient.sendMessage("acidBurn", "Hack the planet!")
-        verify(mockClient).send("PRIVMSG acidBurn :Hack the planet!")
+        verify(mockClient).send(tagMap(), "PRIVMSG", "acidBurn", "Hack the planet!")
     }
 
     @Test
     fun `sendMessage sends correct PRIVMSG message with reply to tag`() {
         mockClient.sendMessage("acidBurn", "Hack the planet!", "abc123")
-        verify(mockClient).send("@+draft/reply=abc123 PRIVMSG acidBurn :Hack the planet!")
+        verify(mockClient).send(tagMap(MessageTag.Reply to "abc123"), "PRIVMSG", "acidBurn", "Hack the planet!")
     }
 
     @Test
     fun `sendCtcp sends correct CTCP message with no arguments`() {
         mockClient.sendCtcp("acidBurn", "ping")
-        verify(mockClient).send("PRIVMSG acidBurn :\u0001PING\u0001")
+        verify(mockClient).send(tagMap(), "PRIVMSG", "acidBurn", "\u0001PING\u0001")
     }
 
     @Test
     fun `sendCtcp sends correct CTCP message with arguments`() {
         mockClient.sendCtcp("acidBurn", "ping", "12345")
-        verify(mockClient).send("PRIVMSG acidBurn :\u0001PING 12345\u0001")
+        verify(mockClient).send(tagMap(), "PRIVMSG", "acidBurn", "\u0001PING 12345\u0001")
     }
 
     @Test
     fun `sendAction sends correct action`() {
         mockClient.sendAction("acidBurn", "hacks the planet")
-        verify(mockClient).send("PRIVMSG acidBurn :\u0001ACTION hacks the planet\u0001")
+        verify(mockClient).send(tagMap(), "PRIVMSG", "acidBurn", "\u0001ACTION hacks the planet\u0001")
     }
 
     @Test
     fun `sendUser sends correct USER message`() {
         mockClient.sendUser("AcidBurn","Kate")
-        verify(mockClient).send("USER AcidBurn 0 * :Kate")
+        verify(mockClient).send("USER", "AcidBurn", "0", "*", "Kate")
     }
 
     @Test
     fun `sendUser sends correct AUTHENTICATE message`() {
         mockClient.sendAuthenticationMessage("SCRAM-MD5")
-        verify(mockClient).send("AUTHENTICATE SCRAM-MD5")
+        verify(mockClient).send("AUTHENTICATE", "SCRAM-MD5")
     }
 
     @Test
     fun `sendUser sends correct blank AUTHENTICATE message`() {
         mockClient.sendAuthenticationMessage()
-        verify(mockClient).send("AUTHENTICATE +")
-    }
-
-    @Test
-    fun `sendWithTag sends message without tags`() {
-        mockClient.sendWithTags(emptyMap(), "PING")
-        verify(mockClient).send("PING")
-    }
-
-    @Test
-    fun `sendWithTag sends message with single tag`() {
-        mockClient.sendWithTags(mapOf(MessageTag.MessageId to "abc"), "PING")
-        verify(mockClient).send("@draft/msgid=abc PING")
-    }
-
-    @Test
-    fun `sendWithTag sends message with multiple tag`() {
-        mockClient.sendWithTags(mapOf(MessageTag.MessageId to "abc", MessageTag.AccountName to "foo"), "PING")
-        verify(mockClient).send("@draft/msgid=abc;account=foo PING")
-    }
-
-    @Test
-    fun `sendWithTag ignores tags with null values`() {
-        mockClient.sendWithTags(mapOf(MessageTag.MessageId to null, MessageTag.AccountName to "foo"), "PING")
-        verify(mockClient).send("@account=foo PING")
+        verify(mockClient).send("AUTHENTICATE", "+")
     }
 
     @Test
     fun `sendTagMessage sends tags`() {
-        mockClient.sendTagMessage("#thegibson", mapOf(MessageTag.MessageId to "id", MessageTag.AccountName to "foo"))
-        verify(mockClient).send("@draft/msgid=id;account=foo TAGMSG #thegibson")
+        val tags = mapOf(MessageTag.MessageId to "id", MessageTag.AccountName to "foo")
+        mockClient.sendTagMessage("#thegibson", tags)
+        verify(mockClient).send(tags, "TAGMSG", "#thegibson")
     }
 
     @Test
     fun `sendTagMessage sends tags with reply ID`() {
-        mockClient.sendTagMessage("#thegibson", mapOf(MessageTag.MessageId to "id", MessageTag.AccountName to "foo"), "otherid")
-        verify(mockClient).send("@draft/msgid=id;account=foo;+draft/reply=otherid TAGMSG #thegibson")
-    }
-
-    @Test
-    fun `escapes tag values`() {
-        assertEquals("\\\\hack\\sthe\\r\\nplanet\\:", "\\hack the\r\nplanet;".escapeTagValue())
+        val tags = mapOf(MessageTag.MessageId to "id", MessageTag.AccountName to "foo")
+        mockClient.sendTagMessage("#thegibson", tags, "otherId")
+        verify(mockClient).send(tags + (MessageTag.Reply to "otherId"), "TAGMSG", "#thegibson")
     }
 
 }

src/test/kotlin/com/dmdirc/ktirc/sasl/ExternalMechanismTest.kt

@@ -14,7 +14,7 @@ internal class ExternalMechanismTest {
 
         mechanism.handleAuthenticationEvent(client, null)
 
-        verify(client).send("AUTHENTICATE +")
+        verify(client).send("AUTHENTICATE", "+")
     }
 
 }

src/test/kotlin/com/dmdirc/ktirc/sasl/PlainMechanismTest.kt

@@ -3,10 +3,7 @@ package com.dmdirc.ktirc.sasl
 import com.dmdirc.ktirc.IrcClient
 import com.dmdirc.ktirc.SaslConfig
 import com.dmdirc.ktirc.model.ServerState
-import com.nhaarman.mockitokotlin2.argumentCaptor
-import com.nhaarman.mockitokotlin2.doReturn
-import com.nhaarman.mockitokotlin2.mock
-import com.nhaarman.mockitokotlin2.verify
+import com.nhaarman.mockitokotlin2.*
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Test
 
@@ -27,11 +24,8 @@ internal class PlainMechanismTest {
         mechanism.handleAuthenticationEvent(ircClient, null)
 
         val captor = argumentCaptor<String>()
-        verify(ircClient).send(captor.capture())
-        val parts = captor.firstValue.split(' ')
-        assertEquals("AUTHENTICATE", parts[0])
-
-        val data = String(parts[1].fromBase64()).split('\u0000')
+        verify(ircClient).send(eq("AUTHENTICATE"), captor.capture())
+        val data = String(captor.firstValue.fromBase64()).split('\u0000')
         assertEquals("acidB", data[0])
         assertEquals("acidB", data[1])
         assertEquals("HackThePlan3t!", data[2])

src/test/kotlin/com/dmdirc/ktirc/sasl/SaslMechanismTest.kt

@@ -31,7 +31,7 @@ internal class SaslMechanismTest {
     fun `base64 encodes authentication data`() {
         val client = mock<IrcClient>()
         client.sendAuthenticationData("abcdef")
-        verify(client).send("AUTHENTICATE YWJjZGVm")
+        verify(client).send("AUTHENTICATE", "YWJjZGVm")
     }
 
     @Test
@@ -39,8 +39,8 @@ internal class SaslMechanismTest {
         val client = mock<IrcClient>()
         client.sendAuthenticationData("abcdef".repeat(120))
         with (inOrder(client)) {
-            verify(client, times(2)).send("AUTHENTICATE ${"YWJjZGVm".repeat(50)}")
-            verify(client).send("AUTHENTICATE ${"YWJjZGVm".repeat(20)}")
+            verify(client, times(2)).send("AUTHENTICATE", "YWJjZGVm".repeat(50))
+            verify(client).send("AUTHENTICATE", "YWJjZGVm".repeat(20))
         }
     }
 
@@ -49,8 +49,8 @@ internal class SaslMechanismTest {
         val client = mock<IrcClient>()
         client.sendAuthenticationData("abcdef".repeat(50))
         with (inOrder(client)) {
-            verify(client).send("AUTHENTICATE ${"YWJjZGVm".repeat(50)}")
-            verify(client).send("AUTHENTICATE +")
+            verify(client).send("AUTHENTICATE", "YWJjZGVm".repeat(50))
+            verify(client).send("AUTHENTICATE", "+")
         }
     }
 

src/test/kotlin/com/dmdirc/ktirc/sasl/ScramMechanismTest.kt

@@ -26,7 +26,7 @@ internal class ScramMechanismTest {
         mechanism.handleAuthenticationEvent(ircClient, "+".toByteArray())
 
         val nonce = (serverState.sasl.mechanismState as ScramState).clientNonce
-        verify(ircClient).send("AUTHENTICATE ${"n,,n=user,r=$nonce".toByteArray().toBase64()}")
+        verify(ircClient).send("AUTHENTICATE", "n,,n=user,r=$nonce".toByteArray().toBase64())
     }
 
     @Test
@@ -40,7 +40,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "m=future".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -54,7 +54,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "e=whoops".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -68,7 +68,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,s=QSXCR+Q6sek8bf92".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -82,7 +82,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,s=QSXCR+Q6sek8bf92,i=leet".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -96,7 +96,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "rs=QSXCR+Q6sek8bf92,i=4096".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -110,7 +110,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,i=4096".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -145,7 +145,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,s=QSXCR+Q6sek8bf92,i=4096".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE ${"c=biws,r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,p=v0X8v3Bz2T0CJGbJQyF0X+HI4Ts=".toByteArray().toBase64()}")
+        verify(ircClient).send("AUTHENTICATE", "c=biws,r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,p=v0X8v3Bz2T0CJGbJQyF0X+HI4Ts=".toByteArray().toBase64())
     }
 
     @Test
@@ -159,7 +159,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "m=future".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -173,7 +173,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "e=whoops".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -190,7 +190,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -207,7 +207,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "v=rmF9pqV8S7suAoZWja4dJRkF=".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE *")
+        verify(ircClient).send("AUTHENTICATE", "*")
     }
 
     @Test
@@ -224,7 +224,7 @@ internal class ScramMechanismTest {
 
         mechanism.handleAuthenticationEvent(ircClient, "v=rmF9pqV8S7suAoZWja4dJRkFsKQ=".toByteArray())
 
-        verify(ircClient).send("AUTHENTICATE +")
+        verify(ircClient).send("AUTHENTICATE", "+")
     }
 
 }