adam/chat-access
1
0
Fork 0
Code Activity

Code cleanups, mostly runBlocking inlining in tests

Browse source
This commit is contained in:
Adam Kruszewski 2024-11-19 19:34:04 +01:00
parent 8681fa3465
commit 4a78a894a4
10 changed files with 745 additions and 822 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
chat-access/src/main/kotlin/interview/chataccess/ChatAccessApplication.kt
View file

@ -10,7 +10,7 @@ import org.springframework.data.r2dbc.repository.config.EnableR2dbcRepositories
@ConfigurationPropertiesScan
@EntityScan("interview.chataccess.dbmodel")
@EnableR2dbcRepositories("interview.chataccess.dbmodel")
open class ChatAccessApplication
class ChatAccessApplication
fun main(args: Array<String>) {
runApplication<ChatAccessApplication>(*args)

7
chat-access/src/main/kotlin/interview/chataccess/controller/MessageController.kt
View file

@ -1,6 +1,5 @@
package interview.chataccess.controller
import interview.chataccess.controller.UtilsRestController.getPrincipal
import interview.chataccess.controller.exceptions.ChatSessionNotFoundException
import interview.chataccess.controller.exceptions.GenericRestException
@ -10,6 +9,7 @@ import interview.chataccess.dto.*
import interview.chataccess.service.ChatService
import interview.chataccess.service.MessageService
import interview.chataccess.utils.logger
import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.async
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.*
@ -59,7 +59,8 @@ class MessageController(
suspend fun sendMessage(@RequestBody input: ChatMessageSendDTO): Flow<ChatMessagePartialReplyDTO> = coroutineScope {
val principal = getPrincipal()
val sId = async {
// Those two coroutines do not need to be LAZY, but it doesn't hurt.
val sId = async(start = CoroutineStart.LAZY) {
chatService
.findChatByIdForUsername(input.chatSessionId, principal)
?: throw ChatSessionNotFoundException(
@ -74,7 +75,7 @@ class MessageController(
// both of the queries asynchronously for all the other cases.
// We take advantage of the coroutines property that error on one of them will cancel
// the parent context (when not using supervisor).
val pastMessages = async {
val pastMessages = async(start = CoroutineStart.LAZY) {
messageService
.findMessagesInChatSessionForUsername(input.chatSessionId, principal)
.map { message ->

6
chat-access/src/main/kotlin/interview/chataccess/controller/UtilsRestController.kt
View file

@ -15,12 +15,12 @@ object UtilsRestController {
.awaitSingle()
.authentication.principal
.let { authPrincipal ->
when {
authPrincipal is User -> {
when (authPrincipal) {
is User -> {
authPrincipal.username
}
authPrincipal is Jwt -> {
is Jwt -> {
authPrincipal
.claims["email"]
.toString()

17
chat-access/src/test/kotlin/interview/chataccess/BaseTests.kt
View file

@ -13,23 +13,18 @@ open class BaseTests {
private var internalCounter: Int = 0
fun genChatSession(): ChatSession {
internalCounter++
return ChatSession.from(
name = internalCounter.toString(),
fun genChatSession() =
ChatSession.from(
name = (++internalCounter).toString(),
model = TESTS_DEFAULT_MODEL,
username = TESTS_DEFAULT_USERNAME
)
}
fun genChatMessage(chatSession: ChatSession): ChatMessage {
internalCounter++
return ChatMessage.from(
fun genChatMessage(chatSession: ChatSession) =
ChatMessage.from(
chatSession,
ChatRoles.USER,
"Test message ${internalCounter}",
"Test message ${(++internalCounter)}",
true
)
}
}

2
chat-access/src/test/kotlin/interview/chataccess/applicationevents/ApplicationStartupEventTests.kt
View file

@ -34,6 +34,7 @@ class ApplicationStartupEventTests : BaseOllamaClientTest() {
.setBody("{\"status\":\"success\"}")
)
}
ApplicationStartupEvent(appConfig, mockWebClient!!)
.loadTestData()
}
@ -57,7 +58,6 @@ class ApplicationStartupEventTests : BaseOllamaClientTest() {
ApplicationStartupEvent(appConfig, mockWebClient!!)
.loadTestData()
}
}

38
chat-access/src/test/kotlin/interview/chataccess/controller/ChatControllerTests.kt
View file

@ -50,8 +50,7 @@ class ChatControllerTests : BaseControllerTests() {
@Test
@DisplayName("ChatControllerTest: fetch models list")
fun getModelsList() {
runBlocking {
fun getModelsList(): Unit = runBlocking {
log.debug("Models in the test scenario: {}", appConfig.availableModels)
getWebTestClient()
@ -70,12 +69,10 @@ class ChatControllerTests : BaseControllerTests() {
}.toTypedArray())
)
}
}
@Test
@DisplayName("ChatControllerTest: fetch chat sessions list")
fun getSessions() {
runBlocking {
fun getSessions(): Unit = runBlocking {
(1..5).map { genChatSession() }
.toTypedArray()
.also { chatSessions ->
@ -106,12 +103,10 @@ class ChatControllerTests : BaseControllerTests() {
.contains(*chatSessions)
}
}
}
@Test
@DisplayName("ChatControllerTest: fetch chat session details")
fun getSessionDetails() {
runBlocking {
fun getSessionDetails(): Unit = runBlocking {
genChatSession()
.apply {
this.id = Random.nextInt(0, Int.MAX_VALUE)
@ -138,12 +133,10 @@ class ChatControllerTests : BaseControllerTests() {
}
}
}
}
@Test
@DisplayName("ChatControllerTest: fetch non-existent chat session details")
fun getSessionDetailsNonExist() {
runBlocking {
fun getSessionDetailsNonExist(): Unit = runBlocking {
genChatSession()
.apply {
// We do not store it, i.e., not mock storing, just simulating,
@ -169,13 +162,10 @@ class ChatControllerTests : BaseControllerTests() {
)
}
}
}
@Test
@DisplayName("ChatControllerTest: delete non-existent chat session")
fun deleteChatSessionNotExist() {
runBlocking {
fun deleteChatSessionNotExist(): Unit = runBlocking {
genChatSession()
.apply {
this.id = Random.nextInt(0, Int.MAX_VALUE)
@ -199,12 +189,10 @@ class ChatControllerTests : BaseControllerTests() {
)
}
}
}
@Test
@DisplayName("ChatControllerTest: delete chat session")
fun deleteChatSession() {
runBlocking {
fun deleteChatSession(): Unit = runBlocking {
genChatSession()
.apply {
this.id = Random.nextInt(0, Int.MAX_VALUE)
@ -226,12 +214,10 @@ class ChatControllerTests : BaseControllerTests() {
.isEqualTo("Deleted")
}
}
}
@Test
@DisplayName("ChatControllerTest: create new chat session with wrong model")
fun createChatSessionWrongModel() {
runBlocking {
fun createChatSessionWrongModel(): Unit = runBlocking {
getWebTestClient()
.put()
.uri("/chat/session")
@ -249,12 +235,10 @@ class ChatControllerTests : BaseControllerTests() {
RestControllerAdvice.messageForUnsupportedMode(TESTS_DEFAULT_WRONG_MODEL)
)
}
}
@Test
@DisplayName("ChatControllerTest: create new chat session with empty name")
fun createChatSessionEmptyName() {
runBlocking {
fun createChatSessionEmptyName(): Unit = runBlocking {
getWebTestClient()
.put()
.uri("/chat/session")
@ -272,12 +256,10 @@ class ChatControllerTests : BaseControllerTests() {
RestControllerAdvice.messageForBlankSessionName()
)
}
}
@Test
@DisplayName("ChatControllerTest: create new chat session with wrong model")
fun createChatSession() {
runBlocking {
fun createChatSession(): Unit = runBlocking {
// This one looked hideous when written functionally, so semi-imperative it goes.
val chatSession = genChatSession()
@ -337,6 +319,4 @@ class ChatControllerTests : BaseControllerTests() {
)
)
}
}
}

29
chat-access/src/test/kotlin/interview/chataccess/controller/MessageControllerTests.kt
View file

@ -47,8 +47,7 @@ class MessageControllerTests : BaseOllamaClientTest() {
@Test
@DisplayName("MessageControllerTest: fetch previous non-existent messages")
fun getPreviousMessagesNotExist() {
runBlocking {
fun getPreviousMessagesNotExist(): Unit = runBlocking {
val chatSessionId = Random.nextInt(0, Int.MAX_VALUE)
given(
@ -67,12 +66,10 @@ class MessageControllerTests : BaseOllamaClientTest() {
RestControllerAdvice.messageForSessionNotFound(chatSessionId)
)
}
}
@Test
@DisplayName("MessageControllerTest: fetch previous non-existent messages")
fun getPrevious() {
runBlocking {
fun getPrevious(): Unit = runBlocking {
val chatSession = genChatSession()
.apply {
this.id = Random.nextInt(0, Int.MAX_VALUE)
@ -107,12 +104,10 @@ class MessageControllerTests : BaseOllamaClientTest() {
.hasSize(chatMessages.size)
.contains(*expectedReply)
}
}
@Test
@DisplayName("MessageControllerTest: send message to non-existent chat session")
fun sendMessageChatNotExist() {
runBlocking {
fun sendMessageChatNotExist(): Unit = runBlocking {
val chatSessionId = Random.nextInt(0, Int.MAX_VALUE)
given(
@ -137,12 +132,10 @@ class MessageControllerTests : BaseOllamaClientTest() {
RestControllerAdvice.messageForSessionNotFound(chatSessionId)
)
}
}
@Test
@DisplayName("MessageControllerTest: send chat message (non-streaming reply)")
fun sendChatMessage() {
runBlocking {
fun sendChatMessage(): Unit = runBlocking {
val chatSession = genChatSession()
.apply {
this.id = Random.nextInt(0, Int.MAX_VALUE)
@ -221,12 +214,10 @@ class MessageControllerTests : BaseOllamaClientTest() {
.hasSize(1)
.contains(*expectedReplies)
}
}
@Test
@DisplayName("MessageControllerTest: send chat message (streaming reply)")
fun sendChatMessageStreaming() {
runBlocking {
fun sendChatMessageStreaming(): Unit = runBlocking {
val chatSession = genChatSession()
.apply {
this.id = Random.nextInt(0, Int.MAX_VALUE)
@ -313,12 +304,10 @@ class MessageControllerTests : BaseOllamaClientTest() {
.hasSize(expectedReplies.size)
.contains(*expectedReplies)
}
}
@Test
@DisplayName("MessageControllerTest: Ollama API returns error")
fun sendChatMessageButGot4xx() {
runBlocking {
fun sendChatMessageButGot4xx(): Unit = runBlocking {
val chatSession = genChatSession()
.apply {
this.id = Random.nextInt(0, Int.MAX_VALUE)
@ -360,12 +349,10 @@ class MessageControllerTests : BaseOllamaClientTest() {
it.startsWith(RestControllerAdvice.messagePrefixForGenericRestException())
}
}
}
@Test
@DisplayName("MessageControllerTest: send chat message (streaming reply with error in middle)")
fun sendChatMessageStreamingError() {
runBlocking {
fun sendChatMessageStreamingError(): Unit = runBlocking {
val chatSession = genChatSession()
.apply {
this.id = Random.nextInt(0, Int.MAX_VALUE)
@ -418,4 +405,4 @@ class MessageControllerTests : BaseOllamaClientTest() {
}
}
}
}

27
chat-access/src/test/kotlin/interview/chataccess/controller/RestSecurityTests.kt
View file

@ -10,10 +10,7 @@ import kotlinx.coroutines.runBlocking
import org.junit.jupiter.api.DisplayName
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
import org.junit.jupiter.api.extension.ExtendWith
import org.mockito.Mock
import org.mockito.Mockito.`when`
import org.mockito.junit.jupiter.MockitoExtension
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.test.autoconfigure.web.reactive.WebFluxTest
import org.springframework.context.annotation.Import
@ -27,7 +24,6 @@ import org.springframework.security.web.server.WebFilterChainProxy
import org.springframework.test.web.reactive.server.WebTestClient
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.RestController
import reactor.core.publisher.Mono
@Import(TestApplicationConfig::class, SecurityConfig::class)
@WebFluxTest(RestSecurityTests.Http200Controller::class)
@ -41,14 +37,8 @@ class RestSecurityTests : BaseControllerTests() {
@RestController
class Http200Controller {
val log = logger()
@GetMapping("/")
suspend fun getOK(): Mono<String> {
return Mono.just("OK")
}
@GetMapping("/user")
suspend fun getUsername(): String = getPrincipal().also { log.debug("Principal: ${it} ") }
}
@ -71,8 +61,7 @@ class RestSecurityTests : BaseControllerTests() {
@Test
@DisplayName("Authentication required")
fun getModelsList() {
runBlocking {
fun getModelsList(): Unit = runBlocking {
getWebTestClient()
.get()
.uri("/")
@ -81,14 +70,12 @@ class RestSecurityTests : BaseControllerTests() {
.expectStatus()
.is4xxClientError
}
}
@Test
@DisplayName("getPrincipal works for mock users")
@WithMockUser(username = TESTS_DEFAULT_USERNAME)
fun getPrincipalForMockUser() {
fun getPrincipalForMockUser(): Unit = runBlocking {
// This one is just to make sure other tests with @WithMockUser do behave.
runBlocking {
getWebTestClient()
.get()
.uri("/user")
@ -97,12 +84,10 @@ class RestSecurityTests : BaseControllerTests() {
.expectStatus()
.is2xxSuccessful
}
}
@Test
@DisplayName("getPrincipal works for mocked OAuth2")
fun getPrincipalForMockOAuth2() {
runBlocking {
fun getPrincipalForMockOAuth2(): Unit = runBlocking {
getWebTestClient()
.mutateWith(
SecurityMockServerConfigurers.mockJwt().jwt { jwt ->
@ -118,16 +103,13 @@ class RestSecurityTests : BaseControllerTests() {
.expectBody(String::class.java)
.isEqualTo(TESTS_DEFAULT_USERNAME)
}
}
@Mock
private val auth: Authentication? = null
@Test
@DisplayName("getPrincipal reacts to non-supported authentication context")
fun getPrincipalWithUnsupportedContext() {
runBlocking {
fun getPrincipalWithUnsupportedContext(): Unit = runBlocking {
assertThrows<IllegalStateException> {
SecurityContextHolder
.getContext()
@ -137,4 +119,3 @@ class RestSecurityTests : BaseControllerTests() {
}
}
}
}

29
chat-access/src/test/kotlin/interview/chataccess/service/ChatServiceTests.kt
View file

@ -45,8 +45,7 @@ class ChatServiceTests : BaseServiceTests() {
@Test
@DisplayName("ChatServiceTest: create chat session")
fun createChatSession() {
runBlocking {
fun createChatSession(): Unit = runBlocking {
genChatSession()
.let { chatSession ->
chatService
@ -59,12 +58,10 @@ class ChatServiceTests : BaseServiceTests() {
}
}
}
}
@Test
@DisplayName("ChatServiceTest: delete chat session")
fun deleteChatSession() {
runBlocking {
fun deleteChatSession(): Unit = runBlocking {
chatService
.save(genChatSession())
.let { chatSession ->
@ -75,23 +72,19 @@ class ChatServiceTests : BaseServiceTests() {
assertNull(chatRepository.findById(chatSession.id!!))
}
}
}
@Test
@DisplayName("ChatServiceTest: delete non-existent chat session")
fun deleteChatSessionNotExist() {
runBlocking {
fun deleteChatSessionNotExist(): Unit = runBlocking {
val id = Random.nextInt(0, Int.MAX_VALUE)
assertFalse(
chatService.delete(id, TESTS_DEFAULT_USERNAME)
)
}
}
@Test
@DisplayName("ChatServiceTest: fetching chat sessions by id and username")
fun findOneByIdAndUsername() {
runBlocking {
fun findOneByIdAndUsername(): Unit = runBlocking {
chatRepository.saveAll(
(1..3).map { genChatSession() }
).onEach { chatSession ->
@ -109,12 +102,10 @@ class ChatServiceTests : BaseServiceTests() {
?: fail("findChatByIdForUsername returned null when shouldn't")
}.lastOrNull()
}
}
@Test
@DisplayName("ChatServiceTest: trying to fetch another user chat session")
fun findOneByIdAndUsernameButDifferentUser() {
runBlocking {
fun findOneByIdAndUsernameButDifferentUser(): Unit = runBlocking {
chatService
.save(genChatSession())
.let { chatSession ->
@ -124,12 +115,10 @@ class ChatServiceTests : BaseServiceTests() {
)
}
}
}
@Test
@DisplayName("ChatServiceTest: trying to fetch non existing chat session")
fun findOneByIdAndUsernameButNonExisting() {
runBlocking {
fun findOneByIdAndUsernameButNonExisting(): Unit = runBlocking {
chatService
.save(genChatSession())
.let { chatSession ->
@ -139,12 +128,10 @@ class ChatServiceTests : BaseServiceTests() {
)
}
}
}
@Test
@DisplayName("ChatServiceTest: checking the order of queried chat sessions for username")
fun findAllByUsernameAndCheckOrder() {
runBlocking {
fun findAllByUsernameAndCheckOrder(): Unit = runBlocking {
// Order should be reversed from the order of inserting into database. Last first.
(1..5).map { genChatSession() }
.also { csList ->
@ -173,5 +160,3 @@ class ChatServiceTests : BaseServiceTests() {
}
}
}
}

12
chat-access/src/test/kotlin/interview/chataccess/service/MessageServiceTests.kt
View file

@ -42,8 +42,7 @@ class MessageServiceTests : BaseServiceTests() {
@Test
@DisplayName("MessageServiceTest: create message")
fun createMessage() {
runBlocking {
fun createMessage(): Unit = runBlocking {
genChatSession()
.let { chatSession ->
genChatMessage(
@ -61,12 +60,10 @@ class MessageServiceTests : BaseServiceTests() {
}
}
}
}
@Test
@DisplayName("MessageServiceTest: find all previous messages and verify order")
fun findAllByChatSessionIdAndUsername() {
runBlocking {
fun findAllByChatSessionIdAndUsername(): Unit = runBlocking {
val chatSession = genChatSession()
.let { chatSession ->
chatService
@ -100,12 +97,10 @@ class MessageServiceTests : BaseServiceTests() {
awaitComplete()
}
}
}
@Test
@DisplayName("MessageServiceTest: whether we can retrieve messages from another user's chat session")
fun findAllForAnotherUser() {
runBlocking {
fun findAllForAnotherUser(): Unit = runBlocking {
val chatSession = genChatSession()
.let { chatSession ->
chatService
@ -129,4 +124,3 @@ class MessageServiceTests : BaseServiceTests() {
}
}
}
}