briar-headless: Address second round of review comments

briar-headless/README.md

@@ -101,7 +101,7 @@ Attention: There can messages of other `type`s where the message `body` is `null
 
 `POST /messages/{contactId}`
 
-The text of the message should be included in the form parameter `message`.
+The text of the message should be included in the form parameter `text`.
 
 ### Listing blog posts
 

briar-headless/src/main/java/org/briarproject/briar/headless/BriarService.kt

@@ -15,8 +15,8 @@ import javax.inject.Singleton
 
 @Immutable
 @Singleton
-internal class BriarService @Inject
-constructor(
+internal class BriarService
+@Inject constructor(
     private val accountManager: AccountManager,
     private val lifecycleManager: LifecycleManager,
     private val passwordStrengthEstimator: PasswordStrengthEstimator

briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessModule.kt

@@ -24,9 +24,11 @@ import org.briarproject.bramble.plugin.tor.CircumventionProvider
 import org.briarproject.bramble.plugin.tor.LinuxTorPluginFactory
 import org.briarproject.bramble.system.JavaSystemModule
 import org.briarproject.bramble.util.StringUtils.fromHexString
-import org.briarproject.briar.headless.event.WebSocketController
-import org.briarproject.briar.headless.event.WebSocketControllerImpl
-import org.briarproject.briar.headless.messaging.MessagingModule
+import org.briarproject.briar.headless.blogs.HeadlessBlogModule
+import org.briarproject.briar.headless.contact.HeadlessContactModule
+import org.briarproject.briar.headless.event.HeadlessEventModule
+import org.briarproject.briar.headless.forums.HeadlessForumModule
+import org.briarproject.briar.headless.messaging.HeadlessMessagingModule
 import java.io.File
 import java.security.GeneralSecurityException
 import java.util.Collections.emptyList
@@ -39,7 +41,11 @@ import javax.net.SocketFactory
         JavaNetworkModule::class,
         JavaSystemModule::class,
         CircumventionModule::class,
-        MessagingModule::class
+        HeadlessBlogModule::class,
+        HeadlessContactModule::class,
+        HeadlessEventModule::class,
+        HeadlessForumModule::class,
+        HeadlessMessagingModule::class
     ]
 )
 internal class HeadlessModule(private val appDir: File) {
@@ -107,12 +113,4 @@ internal class HeadlessModule(private val appDir: File) {
         }
     }
 
-    @Provides
-    @Singleton
-    internal fun provideWebSocketController(
-        webSocketController: WebSocketControllerImpl
-    ): WebSocketController {
-        return webSocketController
-    }
-
 }

briar-headless/src/main/java/org/briarproject/briar/headless/Router.kt

@@ -12,7 +12,8 @@ import org.briarproject.briar.headless.event.WebSocketController
 import org.briarproject.briar.headless.forums.ForumController
 import org.briarproject.briar.headless.messaging.MessagingController
 import java.lang.Runtime.getRuntime
-import java.util.logging.Logger
+import java.util.concurrent.atomic.AtomicBoolean
+import java.util.logging.Logger.getLogger
 import javax.annotation.concurrent.Immutable
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -30,11 +31,12 @@ constructor(
     private val blogController: BlogController
 ) {
 
-    private val logger: Logger = Logger.getLogger(this.javaClass.name)
+    private val logger = getLogger(Router::javaClass.name)
+    private val stopped = AtomicBoolean(false)
 
     fun start(authToken: String, port: Int, debug: Boolean) {
         briarService.start()
-        getRuntime().addShutdownHook(Thread(Runnable { briarService.stop() }))
+        getRuntime().addShutdownHook(Thread(Runnable { stop() }))
 
         val app = Javalin.create()
             .port(port)
@@ -93,8 +95,10 @@ constructor(
     }
 
     private fun stop() {
-        briarService.stop()
-        exitProcess(0)
+        if (!stopped.getAndSet(true)) {
+            briarService.stop()
+            exitProcess(0)
+        }
     }
 
 }

briar-headless/src/main/java/org/briarproject/briar/headless/blogs/BlogController.kt

 package org.briarproject.briar.headless.blogs
 
-import io.javalin.BadRequestResponse
 import io.javalin.Context
-import org.briarproject.bramble.api.identity.IdentityManager
-import org.briarproject.bramble.api.system.Clock
-import org.briarproject.bramble.util.StringUtils
-import org.briarproject.briar.api.blog.BlogConstants.MAX_BLOG_POST_BODY_LENGTH
-import org.briarproject.briar.api.blog.BlogManager
-import org.briarproject.briar.api.blog.BlogPostFactory
-import javax.annotation.concurrent.Immutable
-import javax.inject.Inject
-import javax.inject.Singleton
 
-@Immutable
-@Singleton
-class BlogController @Inject
-constructor(
-    private val blogManager: BlogManager,
-    private val blogPostFactory: BlogPostFactory,
-    private val identityManager: IdentityManager,
-    private val clock: Clock
-) {
+interface BlogController {
 
-    fun listPosts(ctx: Context): Context {
-        val posts = blogManager.blogs.flatMap { blog ->
-            blogManager.getPostHeaders(blog.id).map { header ->
-                val body = blogManager.getPostBody(header.id)
-                header.output(body)
-            }
-        }.sortedBy { it.timestampReceived }
-        return ctx.json(posts)
-    }
+    fun listPosts(ctx: Context): Context
 
-    fun createPost(ctx: Context): Context {
-        val text = ctx.formParam("text")
-        if (text == null || text.isEmpty())
-            throw BadRequestResponse("Expecting blog post text")
-        if (StringUtils.toUtf8(text).size > MAX_BLOG_POST_BODY_LENGTH)
-            throw BadRequestResponse("Too long blog post text")
-
-        val author = identityManager.localAuthor
-        val blog = blogManager.getPersonalBlog(author)
-        val now = clock.currentTimeMillis()
-        val post = blogPostFactory.createBlogPost(blog.id, now, null, author, text)
-        blogManager.addLocalPost(post)
-        val header = blogManager.getPostHeader(blog.id, post.message.id)
-        return ctx.json(header.output(text))
-    }
+    fun createPost(ctx: Context): Context
 
 }

briar-headless/src/main/java/org/briarproject/briar/headless/blogs/BlogControllerImpl.kt

+package org.briarproject.briar.headless.blogs
+
+import io.javalin.BadRequestResponse
+import io.javalin.Context
+import org.briarproject.bramble.api.identity.IdentityManager
+import org.briarproject.bramble.api.system.Clock
+import org.briarproject.bramble.util.StringUtils
+import org.briarproject.briar.api.blog.BlogConstants
+import org.briarproject.briar.api.blog.BlogManager
+import org.briarproject.briar.api.blog.BlogPostFactory
+import javax.annotation.concurrent.Immutable
+import javax.inject.Inject
+import javax.inject.Singleton
+
+@Immutable
+@Singleton
+internal class BlogControllerImpl
+@Inject constructor(
+    private val blogManager: BlogManager,
+    private val blogPostFactory: BlogPostFactory,
+    private val identityManager: IdentityManager,
+    private val clock: Clock
+) : BlogController {
+
+    override fun listPosts(ctx: Context): Context {
+        val posts = blogManager.blogs.flatMap { blog ->
+            blogManager.getPostHeaders(blog.id).map { header ->
+                val body = blogManager.getPostBody(header.id)
+                header.output(body)
+            }
+        }.sortedBy { it.timestampReceived }
+        return ctx.json(posts)
+    }
+
+    override fun createPost(ctx: Context): Context {
+        val text = ctx.formParam("text")
+        if (text == null || text.isEmpty())
+            throw BadRequestResponse("Expecting blog post text")
+        if (StringUtils.utf8IsTooLong(text, BlogConstants.MAX_BLOG_POST_BODY_LENGTH))
+            throw BadRequestResponse("Too long blog post text")
+
+        val author = identityManager.localAuthor
+        val blog = blogManager.getPersonalBlog(author)
+        val now = clock.currentTimeMillis()
+        val post = blogPostFactory.createBlogPost(blog.id, now, null, author, text)
+        blogManager.addLocalPost(post)
+        val header = blogManager.getPostHeader(blog.id, post.message.id)
+        return ctx.json(header.output(text))
+    }
+
+}

briar-headless/src/main/java/org/briarproject/briar/headless/blogs/HeadlessBlogModule.kt

+package org.briarproject.briar.headless.blogs
+
+import dagger.Module
+import dagger.Provides
+import javax.inject.Singleton
+
+@Module
+class HeadlessBlogModule {
+
+    @Provides
+    @Singleton
+    internal fun provideBlogController(blogController: BlogControllerImpl): BlogController {
+        return blogController
+    }
+
+}

briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactController.kt

 package org.briarproject.briar.headless.contact
 
 import io.javalin.Context
-import org.briarproject.bramble.api.contact.ContactManager
-import javax.annotation.concurrent.Immutable
-import javax.inject.Inject
-import javax.inject.Singleton
 
-@Immutable
-@Singleton
-class ContactController @Inject
-constructor(private val contactManager: ContactManager) {
+interface ContactController {
 
-    fun list(ctx: Context): Context {
-        val contacts = contactManager.activeContacts.map { contact ->
-            contact.output()
-        }
-        return ctx.json(contacts)
-    }
+    fun list(ctx: Context): Context
 
 }

briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactControllerImpl.kt

+package org.briarproject.briar.headless.contact
+
+import io.javalin.Context
+import org.briarproject.bramble.api.contact.ContactManager
+import javax.annotation.concurrent.Immutable
+import javax.inject.Inject
+import javax.inject.Singleton
+
+@Immutable
+@Singleton
+internal class ContactControllerImpl
+@Inject constructor(private val contactManager: ContactManager) : ContactController {
+
+    override fun list(ctx: Context): Context {
+        val contacts = contactManager.activeContacts.map { contact ->
+            contact.output()
+        }
+        return ctx.json(contacts)
+    }
+
+}

briar-headless/src/main/java/org/briarproject/briar/headless/contact/HeadlessContactModule.kt

+package org.briarproject.briar.headless.contact
+
+import dagger.Module
+import dagger.Provides
+import javax.inject.Singleton
+
+@Module
+class HeadlessContactModule {
+
+    @Provides
+    @Singleton
+    internal fun provideContactController(contactController: ContactControllerImpl): ContactController {
+        return contactController
+    }
+
+}

briar-headless/src/main/java/org/briarproject/briar/headless/event/HeadlessEventModule.kt

+package org.briarproject.briar.headless.event
+
+import dagger.Module
+import dagger.Provides
+import javax.inject.Singleton
+
+@Module
+class HeadlessEventModule {
+
+    @Provides
+    @Singleton
+    internal fun provideWebSocketController(webSocketController: WebSocketControllerImpl): WebSocketController {
+        return webSocketController
+    }
+
+}

briar-headless/src/main/java/org/briarproject/briar/headless/event/WebSocketController.kt

 package org.briarproject.briar.headless.event
 
 import io.javalin.websocket.WsSession
+import org.briarproject.bramble.api.lifecycle.IoExecutor
+import javax.annotation.concurrent.ThreadSafe
 
+@ThreadSafe
 interface WebSocketController {
 
     val sessions: MutableSet<WsSession>
 
+    /**
+     * Sends an event to all open sessions using the [IoExecutor].
+     */
     fun sendEvent(name: String, obj: Any)
 
 }

briar-headless/src/main/java/org/briarproject/briar/headless/event/WebSocketControllerImpl.kt

@@ -2,26 +2,28 @@ package org.briarproject.briar.headless.event
 
 import io.javalin.json.JavalinJson.toJson
 import io.javalin.websocket.WsSession
+import org.briarproject.bramble.api.lifecycle.IoExecutor
 import org.briarproject.bramble.util.LogUtils.logException
 import org.eclipse.jetty.websocket.api.WebSocketException
 import java.io.IOException
 import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.Executor
 import java.util.logging.Level
-import java.util.logging.Logger
+import java.util.logging.Logger.getLogger
 import javax.annotation.concurrent.Immutable
 import javax.inject.Inject
 import javax.inject.Singleton
 
 @Immutable
 @Singleton
-internal class WebSocketControllerImpl @Inject constructor() :
-    WebSocketController {
+internal class WebSocketControllerImpl
+@Inject constructor(@IoExecutor private val ioExecutor: Executor) : WebSocketController {
 
-    private val logger: Logger = Logger.getLogger(this.javaClass.name)
+    private val logger = getLogger(WebSocketControllerImpl::javaClass.name)
 
     override val sessions: MutableSet<WsSession> = ConcurrentHashMap.newKeySet<WsSession>()
 
-    override fun sendEvent(name: String, obj: Any) {
+    override fun sendEvent(name: String, obj: Any) = ioExecutor.execute {
         sessions.forEach { session ->
             val event = OutputEvent(name, obj)
             try {

briar-headless/src/main/java/org/briarproject/briar/headless/forums/ForumController.kt

 package org.briarproject.briar.headless.forums
 
-import io.javalin.BadRequestResponse
 import io.javalin.Context
-import org.briarproject.bramble.util.StringUtils
-import org.briarproject.briar.api.forum.ForumConstants.MAX_FORUM_NAME_LENGTH
-import org.briarproject.briar.api.forum.ForumManager
-import javax.annotation.concurrent.Immutable
-import javax.inject.Inject
-import javax.inject.Singleton
 
-@Immutable
-@Singleton
-class ForumController @Inject
-constructor(private val forumManager: ForumManager) {
+interface ForumController {
 
-    fun list(ctx: Context): Context {
-        return ctx.json(forumManager.forums.output())
-    }
+    fun list(ctx: Context): Context
 
-    fun create(ctx: Context): Context {
-        val name = ctx.formParam("name")
-        if (name == null || name.isNullOrEmpty())
-            throw BadRequestResponse("Expecting Forum Name")
-        if (StringUtils.toUtf8(name).size > MAX_FORUM_NAME_LENGTH)
-            throw BadRequestResponse("Forum name is too long")
-        return ctx.json(forumManager.addForum(name).output())
-    }
+    fun create(ctx: Context): Context
 
 }

briar-headless/src/main/java/org/briarproject/briar/headless/forums/ForumControllerImpl.kt

+package org.briarproject.briar.headless.forums
+
+import io.javalin.BadRequestResponse
+import io.javalin.Context
+import org.briarproject.bramble.util.StringUtils
+import org.briarproject.briar.api.forum.ForumConstants
+import org.briarproject.briar.api.forum.ForumManager
+import javax.annotation.concurrent.Immutable
+import javax.inject.Inject
+import javax.inject.Singleton
+
+@Immutable
+@Singleton
+internal class ForumControllerImpl
+@Inject constructor(private val forumManager: ForumManager) : ForumController {
+
+    override fun list(ctx: Context): Context {
+        return ctx.json(forumManager.forums.output())
+    }
+
+    override fun create(ctx: Context): Context {
+        val name = ctx.formParam("text")
+        if (name == null || name.isNullOrEmpty())
+            throw BadRequestResponse("Expecting Forum Name")
+        if (StringUtils.utf8IsTooLong(name, ForumConstants.MAX_FORUM_NAME_LENGTH))
+            throw BadRequestResponse("Forum name is too long")
+        return ctx.json(forumManager.addForum(name).output())
+    }
+
+}

briar-headless/src/main/java/org/briarproject/briar/headless/forums/HeadlessForumModule.kt

+package org.briarproject.briar.headless.forums
+
+import dagger.Module
+import dagger.Provides
+import javax.inject.Singleton
+
+@Module
+class HeadlessForumModule {
+
+    @Provides
+    @Singleton
+    internal fun provideForumController(forumController: ForumControllerImpl): ForumController {
+        return forumController
+    }
+
+}

briar-headless/src/main/java/org/briarproject/briar/headless/messaging/MessagingModule.kt → briar-headless/src/main/java/org/briarproject/briar/headless/messaging/HeadlessMessagingModule.kt

@@ -6,7 +6,7 @@ import org.briarproject.bramble.api.event.EventBus
 import javax.inject.Singleton
 
 @Module
-class MessagingModule {
+class HeadlessMessagingModule {
 
     @Provides
     @Singleton

briar-headless/src/main/java/org/briarproject/briar/headless/messaging/MessagingControllerImpl.kt

@@ -11,6 +11,7 @@ import org.briarproject.bramble.api.db.NoSuchContactException
 import org.briarproject.bramble.api.event.Event
 import org.briarproject.bramble.api.event.EventListener
 import org.briarproject.bramble.api.system.Clock
+import org.briarproject.bramble.util.StringUtils.utf8IsTooLong
 import org.briarproject.briar.api.messaging.*
 import org.briarproject.briar.api.messaging.MessagingConstants.MAX_PRIVATE_MESSAGE_BODY_LENGTH
 import org.briarproject.briar.api.messaging.event.PrivateMessageReceivedEvent
@@ -26,8 +27,8 @@ internal const val EVENT_PRIVATE_MESSAGE =
 
 @Immutable
 @Singleton
-internal class MessagingControllerImpl @Inject
-constructor(
+internal class MessagingControllerImpl
+@Inject constructor(
     private val messagingManager: MessagingManager,
     private val conversationManager: ConversationManager,
     private val privateMessageFactory: PrivateMessageFactory,
@@ -55,10 +56,10 @@ constructor(
     override fun write(ctx: Context): Context {
         val contact = getContact(ctx)
 
-        val message = ctx.formParam("message")
+        val message = ctx.formParam("text")
         if (message == null || message.isEmpty())
             throw BadRequestResponse("Expecting Message text")
-        if (message.length > MAX_PRIVATE_MESSAGE_BODY_LENGTH)
+        if (utf8IsTooLong(message, MAX_PRIVATE_MESSAGE_BODY_LENGTH))
             throw BadRequestResponse("Message text too large")
 
         val group = messagingManager.getContactGroup(contact)
@@ -71,7 +72,7 @@ constructor(
 
     override fun eventOccurred(e: Event) {
         when (e) {
-            is PrivateMessageReceivedEvent<*> -> dbExecutor.run {
+            is PrivateMessageReceivedEvent<*> -> dbExecutor.execute {
                 val body = messagingManager.getMessageBody(e.messageHeader.id)
                 webSocketController.sendEvent(EVENT_PRIVATE_MESSAGE, e.output(body))
             }

briar-headless/src/main/java/org/briarproject/briar/headless/messaging/OutputPrivateMessage.kt

@@ -12,7 +12,7 @@ internal abstract class OutputPrivateMessage(
     open val body: String?
 ) {
 
-    open val type: String get() = throw NotImplementedError()
+    abstract val type: String
     val contactId: Int get() = iContactId.int
     val timestamp: Long get() = iHeader.timestamp
     val read: Boolean get() = iHeader.isRead

briar-headless/src/test/java/org/briarproject/briar/headless/blogs/BlogControllerTest.kt

@@ -23,7 +23,7 @@ internal class BlogControllerTest : ControllerTest() {
     private val blogPostFactory = mockk<BlogPostFactory>()
 
     private val controller =
-        BlogController(blogManager, blogPostFactory, identityManager, clock)
+        BlogControllerImpl(blogManager, blogPostFactory, identityManager, clock)
 
     private val blog = Blog(group, author, false)
     private val parentId: MessageId? = null

briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerTest.kt

@@ -9,7 +9,7 @@ import org.junit.jupiter.api.Test
 
 internal class ContactControllerTest : ControllerTest() {
 
-    private val controller = ContactController(contactManager)
+    private val controller = ContactControllerImpl(contactManager)
 
     @Test
     fun testEmptyContactList() {

briar-headless/src/test/java/org/briarproject/briar/headless/event/WebSocketControllerTest.kt

@@ -3,17 +3,22 @@ package org.briarproject.briar.headless.event
 import io.javalin.json.JavalinJson.toJson
 import io.javalin.websocket.WsSession
 import io.mockk.*
+import org.briarproject.bramble.test.ImmediateExecutor
 import org.briarproject.briar.api.messaging.PrivateMessageHeader
 import org.briarproject.briar.api.messaging.event.PrivateMessageReceivedEvent
 import org.briarproject.briar.headless.ControllerTest
 import org.briarproject.briar.headless.messaging.EVENT_PRIVATE_MESSAGE
 import org.briarproject.briar.headless.messaging.output
+import org.eclipse.jetty.websocket.api.WebSocketException
 import org.junit.jupiter.api.Test
+import java.io.IOException
 
 internal class WebSocketControllerTest : ControllerTest() {
 
-    private val session = mockk<WsSession>()
-    private val controller = WebSocketControllerImpl()
+    private val session1 = mockk<WsSession>()
+    private val session2 = mockk<WsSession>()
+
+    private val controller = WebSocketControllerImpl(ImmediateExecutor())
 
     private val header =
         PrivateMessageHeader(message.id, group.id, timestamp, true, true, true, true)
@@ -21,17 +26,40 @@ internal class WebSocketControllerTest : ControllerTest() {
     private val outputEvent = OutputEvent(EVENT_PRIVATE_MESSAGE, event.output(body))
 
     @Test
-    fun testSessionSend() {
+    fun testSendEvent() {
         val slot = CapturingSlot<String>()
 
-        every { session.send(capture(slot)) } just Runs
+        every { session1.send(capture(slot)) } just Runs
 
-        controller.sessions.add(session)
+        controller.sessions.add(session1)
         controller.sendEvent(EVENT_PRIVATE_MESSAGE, event.output(body))
 
         assertJsonEquals(slot.captured, outputEvent)
     }
 
+    @Test
+    fun testSendEventIOException() {
+        testSendEventException(IOException())
+    }
+
+    @Test
+    fun testSendEventWebSocketException() {
+        testSendEventException(WebSocketException())
+    }
+
+    private fun testSendEventException(throwable: Throwable) {
+        val slot = CapturingSlot<String>()
+
+        every { session1.send(capture(slot)) } throws throwable
+        every { session2.send(capture(slot)) } just Runs
+
+        controller.sessions.add(session1)
+        controller.sessions.add(session2)
+        controller.sendEvent(EVENT_PRIVATE_MESSAGE, event.output(body))
+
+        verify { session2.send(slot.captured) }
+    }
+
     @Test
     fun testOutputPrivateMessageReceivedEvent() {
         val json = """

briar-headless/src/test/java/org/briarproject/briar/headless/forums/ForumControllerTest.kt

@@ -16,7 +16,7 @@ internal class ForumControllerTest : ControllerTest() {
 
     private val forumManager = mockk<ForumManager>()