Bump to leakycauldron 1.14.x

Have Loaders use suspend functions instead of CompletableFutures which integrates with the scope cancellation work in LC 1.14.x Note that due to the way `java-dataloader` works, the model fetcher functions can't be async/suspend functions, as we need the dataloader.load() invocation to happen synchronously to ensure they preceed any dispatch() calls; otherwise queries can hang forever, similar to: graphql-java/java-dataloader#54 Add tests to validate the suspend function implemented loaders and existing model fetchers work properly.
josephlbarnett · Sep 16, 2020 · b0e1201 · b0e1201
1 parent 06421ca
commit b0e1201
Show file tree

Hide file tree

Showing 22 changed files with 751 additions and 91 deletions.
diff --git a/pom.xml b/pom.xml
@@ -9,11 +9,11 @@
     <parent>
         <groupId>com.trib3</groupId>
         <artifactId>parent-pom</artifactId>
-        <version>[1.12.1,1.13-SNAPSHOT)</version>
+        <version>[1.14.1,1.15-SNAPSHOT)</version>
     </parent>
 
     <properties>
-        <version.leakycauldron>[1.12.1,1.13-SNAPSHOT)</version.leakycauldron>
+        <version.leakycauldron>[1.14.1,1.15-SNAPSHOT)</version.leakycauldron>
     </properties>
 
     <modules>

diff --git a/server/pom.xml b/server/pom.xml
@@ -146,10 +146,6 @@
             <groupId>org.jetbrains.kotlinx</groupId>
             <artifactId>kotlinx-coroutines-jdk8</artifactId>
         </dependency>
-        <dependency>
-            <groupId>org.jetbrains.kotlinx</groupId>
-            <artifactId>kotlinx-coroutines-slf4j</artifactId>
-        </dependency>
         <dependency>
             <groupId>com.graphql-java</groupId>
             <artifactId>java-dataloader</artifactId>

diff --git a/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/BatchQuestionLoader.kt b/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/BatchQuestionLoader.kt
@@ -2,23 +2,15 @@ package com.joe.quizzy.server.graphql.dataloaders
 
 import com.joe.quizzy.api.models.Question
 import com.joe.quizzy.persistence.api.QuestionDAO
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.future.future
-import kotlinx.coroutines.slf4j.MDCContext
-import org.dataloader.MappedBatchLoader
+import org.dataloader.BatchLoaderEnvironment
 import java.util.UUID
-import java.util.concurrent.CompletionStage
 
 /**
  * Batch load Questions by ID
  */
 class BatchQuestionLoader(private val questionDAO: QuestionDAO) :
-    MappedBatchLoader<UUID, Question> {
-    override fun load(keys: Set<UUID>): CompletionStage<Map<UUID, Question>> {
-        return CoroutineScope(Dispatchers.IO + MDCContext())
-            .future {
-                questionDAO.get(keys.toList()).associateBy { it.id!! }
-            }
+    CoroutineMappedBatchLoader<UUID, Question>() {
+    override suspend fun loadSuspend(keys: Set<UUID>, environment: BatchLoaderEnvironment): Map<UUID, Question> {
+        return questionDAO.get(keys.toList()).associateBy { it.id!! }
     }
 }
diff --git a/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/BatchUserLoader.kt b/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/BatchUserLoader.kt
@@ -2,23 +2,15 @@ package com.joe.quizzy.server.graphql.dataloaders
 
 import com.joe.quizzy.api.models.User
 import com.joe.quizzy.persistence.api.UserDAO
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.future.future
-import kotlinx.coroutines.slf4j.MDCContext
-import org.dataloader.MappedBatchLoader
+import org.dataloader.BatchLoaderEnvironment
 import java.util.UUID
-import java.util.concurrent.CompletionStage
 
 /**
  * Batch load Users by ID
  */
 class BatchUserLoader(private val userDAO: UserDAO) :
-    MappedBatchLoader<UUID, User> {
-    override fun load(keys: Set<UUID>): CompletionStage<Map<UUID, User>> {
-        return CoroutineScope(Dispatchers.IO + MDCContext())
-            .future {
-                userDAO.get(keys.toList()).associateBy { it.id!! }
-            }
+    CoroutineMappedBatchLoader<UUID, User>() {
+    override suspend fun loadSuspend(keys: Set<UUID>, environment: BatchLoaderEnvironment): Map<UUID, User> {
+        return userDAO.get(keys.toList()).associateBy { it.id!! }
     }
 }
diff --git a/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/BulkInstanceLoader.kt b/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/BulkInstanceLoader.kt
@@ -2,23 +2,15 @@ package com.joe.quizzy.server.graphql.dataloaders
 
 import com.joe.quizzy.api.models.Instance
 import com.joe.quizzy.persistence.api.InstanceDAO
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.future.future
-import kotlinx.coroutines.slf4j.MDCContext
-import org.dataloader.MappedBatchLoader
+import org.dataloader.BatchLoaderEnvironment
 import java.util.UUID
-import java.util.concurrent.CompletionStage
 
 /**
  * Batch load Instance by Id
  */
 class BulkInstanceLoader(private val instanceDAO: InstanceDAO) :
-    MappedBatchLoader<UUID, Instance> {
-    override fun load(keys: Set<UUID>): CompletionStage<Map<UUID, Instance>> {
-        return CoroutineScope(Dispatchers.IO + MDCContext())
-            .future {
-                instanceDAO.get(keys.toList()).associateBy { it.id!! }
-            }
+    CoroutineMappedBatchLoader<UUID, Instance>() {
+    override suspend fun loadSuspend(keys: Set<UUID>, environment: BatchLoaderEnvironment): Map<UUID, Instance> {
+        return instanceDAO.get(keys.toList()).associateBy { it.id!! }
     }
 }
diff --git a/...r/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/CoroutineMappedBatchLoader.kt b/...r/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/CoroutineMappedBatchLoader.kt
@@ -0,0 +1,23 @@
+package com.joe.quizzy.server.graphql.dataloaders
+
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.GlobalScope
+import kotlinx.coroutines.future.future
+import org.dataloader.BatchLoaderEnvironment
+import org.dataloader.MappedBatchLoaderWithContext
+import java.util.concurrent.CompletionStage
+
+abstract class CoroutineMappedBatchLoader<K, V> : MappedBatchLoaderWithContext<K, V> {
+
+    private fun scope(environment: BatchLoaderEnvironment): CoroutineScope {
+        return (environment.getContext() as? CoroutineScope) ?: GlobalScope
+    }
+
+    abstract suspend fun loadSuspend(keys: Set<K>, environment: BatchLoaderEnvironment): Map<K, V>
+
+    override fun load(keys: Set<K>, environment: BatchLoaderEnvironment): CompletionStage<Map<K, V>> {
+        return scope(environment).future {
+            loadSuspend(keys, environment)
+        }
+    }
+}
diff --git a/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/QuestionResponseLoader.kt b/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/QuestionResponseLoader.kt
@@ -4,32 +4,23 @@ import com.joe.quizzy.api.models.Response
 import com.joe.quizzy.persistence.api.ResponseDAO
 import com.joe.quizzy.server.auth.UserPrincipal
 import com.trib3.graphql.resources.GraphQLResourceContext
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.future.future
-import kotlinx.coroutines.slf4j.MDCContext
 import org.dataloader.BatchLoaderEnvironment
-import org.dataloader.MappedBatchLoaderWithContext
 import java.util.UUID
-import java.util.concurrent.CompletionStage
 
 /**
  * Batch load Question ID -> Response for context User
  */
 class QuestionResponseLoader(private val responseDAO: ResponseDAO) :
-    MappedBatchLoaderWithContext<UUID, Response> {
-    override fun load(
+    CoroutineMappedBatchLoader<UUID, Response>() {
+    override suspend fun loadSuspend(
         keys: Set<UUID>,
         environment: BatchLoaderEnvironment
-    ): CompletionStage<Map<UUID, Response>> {
-        return CoroutineScope(Dispatchers.IO + MDCContext())
-            .future {
-                val principal = environment.getContext<GraphQLResourceContext>().principal
-                if (principal is UserPrincipal) {
-                    responseDAO.byUserQuestions(principal.user.id!!, keys.toList())
-                } else {
-                    emptyMap()
-                }
-            }
+    ): Map<UUID, Response> {
+        val principal = environment.getContext<GraphQLResourceContext>().principal
+        return if (principal is UserPrincipal) {
+            responseDAO.byUserQuestions(principal.user.id!!, keys.toList())
+        } else {
+            emptyMap()
+        }
     }
 }
diff --git a/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/ResponseGradeLoader.kt b/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/ResponseGradeLoader.kt
@@ -2,23 +2,15 @@ package com.joe.quizzy.server.graphql.dataloaders
 
 import com.joe.quizzy.api.models.Grade
 import com.joe.quizzy.persistence.api.GradeDAO
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.future.future
-import kotlinx.coroutines.slf4j.MDCContext
-import org.dataloader.MappedBatchLoader
+import org.dataloader.BatchLoaderEnvironment
 import java.util.UUID
-import java.util.concurrent.CompletionStage
 
 /**
  * Batch load Response ID -> Grade
  */
 class ResponseGradeLoader(private val gradeDAO: GradeDAO) :
-    MappedBatchLoader<UUID, Grade> {
-    override fun load(keys: Set<UUID>): CompletionStage<Map<UUID, Grade>> {
-        return CoroutineScope(Dispatchers.IO + MDCContext())
-            .future {
-                gradeDAO.forResponses(keys.toList())
-            }
+    CoroutineMappedBatchLoader<UUID, Grade>() {
+    override suspend fun loadSuspend(keys: Set<UUID>, environment: BatchLoaderEnvironment): Map<UUID, Grade> {
+        return gradeDAO.forResponses(keys.toList())
     }
 }
diff --git a/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/UserGradeLoader.kt b/server/src/main/kotlin/com/joe/quizzy/server/graphql/dataloaders/UserGradeLoader.kt
@@ -2,23 +2,15 @@ package com.joe.quizzy.server.graphql.dataloaders
 
 import com.joe.quizzy.api.models.Grade
 import com.joe.quizzy.persistence.api.GradeDAO
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.future.future
-import kotlinx.coroutines.slf4j.MDCContext
-import org.dataloader.MappedBatchLoader
+import org.dataloader.BatchLoaderEnvironment
 import java.util.UUID
-import java.util.concurrent.CompletionStage
 
 /**
  * Batch load User ID -> List<Grade>
  */
 class UserGradeLoader(private val gradeDAO: GradeDAO) :
-    MappedBatchLoader<UUID, List<Grade>> {
-    override fun load(userIds: Set<UUID>): CompletionStage<Map<UUID, List<Grade>>> {
-        return CoroutineScope(Dispatchers.IO + MDCContext())
-            .future {
-                gradeDAO.forUsers(userIds.toList())
-            }
+    CoroutineMappedBatchLoader<UUID, List<Grade>>() {
+    override suspend fun loadSuspend(keys: Set<UUID>, environment: BatchLoaderEnvironment): Map<UUID, List<Grade>> {
+        return gradeDAO.forUsers(keys.toList())
     }
 }
diff --git a/server/src/main/kotlin/com/joe/quizzy/server/graphql/models/ApiResponse.kt b/server/src/main/kotlin/com/joe/quizzy/server/graphql/models/ApiResponse.kt
@@ -33,7 +33,7 @@ data class ApiResponse(
         val principal = context.principal
         if (principal is UserPrincipal) {
             return dfe.getDataLoader<UUID, User>("batchusers").load(userId).thenApply {
-                ApiUser(it)
+                it?.let(::ApiUser)
             }
         }
         return CompletableFuture.completedFuture(null)
@@ -42,7 +42,8 @@ data class ApiResponse(
     fun question(context: GraphQLResourceContext, dfe: DataFetchingEnvironment): CompletableFuture<ApiQuestion?> {
         val principal = context.principal
         if (principal is UserPrincipal) {
-            return dfe.getDataLoader<UUID, Question>("batchquestions").load(questionId).thenApply { ApiQuestion(it) }
+            return dfe.getDataLoader<UUID, Question>("batchquestions").load(questionId)
+                .thenApply { it?.let(::ApiQuestion) }
         }
         return CompletableFuture.completedFuture(null)
     }

diff --git a/server/src/test/kotlin/com/joe/quizzy/server/graphql/MutationTest.kt b/server/src/test/kotlin/com/joe/quizzy/server/graphql/MutationTest.kt
@@ -268,7 +268,7 @@ class MutationTest {
         }.test {
             assertThat(
                 mutation.changePassword(
-                    GraphQLResourceContext(UserPrincipal(user.copy(id = UUID.randomUUID()), null), null),
+                    GraphQLResourceContext(UserPrincipal(user.copy(id = UUID.randomUUID()), null)),
                     "pass",
                     "newpass"
                 )
@@ -287,7 +287,7 @@ class MutationTest {
         }.test {
             assertThat(
                 mutation.changePassword(
-                    GraphQLResourceContext(UserPrincipal(user.copy(email = "user2"), null), null),
+                    GraphQLResourceContext(UserPrincipal(user.copy(email = "user2"), null)),
                     "pass",
                     "newpass"
                 )
@@ -305,7 +305,7 @@ class MutationTest {
         }.test {
             assertThat(
                 mutation.changePassword(
-                    GraphQLResourceContext(UserPrincipal(user.copy(email = "user3"), null), null),
+                    GraphQLResourceContext(UserPrincipal(user.copy(email = "user3"), null)),
                     "pass",
                     "newpass"
                 )

diff --git a/server/src/test/kotlin/com/joe/quizzy/server/graphql/dataloaders/BatchQuestionLoaderTest.kt b/server/src/test/kotlin/com/joe/quizzy/server/graphql/dataloaders/BatchQuestionLoaderTest.kt
@@ -0,0 +1,35 @@
+package com.joe.quizzy.server.graphql.dataloaders
+
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import com.joe.quizzy.api.models.Question
+import com.joe.quizzy.persistence.api.QuestionDAO
+import com.trib3.testing.LeakyMock
+import kotlinx.coroutines.future.await
+import kotlinx.coroutines.runBlocking
+import org.dataloader.BatchLoaderEnvironment
+import org.easymock.EasyMock
+import org.testng.annotations.Test
+import java.time.OffsetDateTime
+import java.util.UUID
+
+class BatchQuestionLoaderTest {
+    @Test
+    fun testQuestionLoader() = runBlocking {
+        val questionDAO = LeakyMock.mock<QuestionDAO>()
+        val mockEnv = LeakyMock.mock<BatchLoaderEnvironment>()
+        val loader = BatchQuestionLoader(questionDAO)
+        val now = OffsetDateTime.now()
+        val questions = listOf(
+            Question(UUID.randomUUID(), UUID.randomUUID(), "q1", "a1", "r1", now, now),
+            Question(UUID.randomUUID(), UUID.randomUUID(), "q2", "a2", "r2", now, now),
+            Question(UUID.randomUUID(), UUID.randomUUID(), "q3", "a3", "r3", now, now)
+        )
+        EasyMock.expect(questionDAO.get(EasyMock.anyObject<List<UUID>>() ?: listOf())).andReturn(questions)
+        EasyMock.expect(mockEnv.getContext<Any?>()).andReturn(null)
+        EasyMock.replay(questionDAO, mockEnv)
+        val qs = loader.load(questions.mapNotNull { it.id }.toSet(), mockEnv).await()
+        assertThat(qs).isEqualTo(questions.associateBy { it.id })
+        EasyMock.verify(questionDAO, mockEnv)
+    }
+}
diff --git a/server/src/test/kotlin/com/joe/quizzy/server/graphql/dataloaders/BatchUserLoaderTest.kt b/server/src/test/kotlin/com/joe/quizzy/server/graphql/dataloaders/BatchUserLoaderTest.kt
@@ -0,0 +1,32 @@
+package com.joe.quizzy.server.graphql.dataloaders
+
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import com.joe.quizzy.api.models.User
+import com.joe.quizzy.persistence.api.UserDAO
+import com.trib3.testing.LeakyMock
+import kotlinx.coroutines.future.await
+import kotlinx.coroutines.runBlocking
+import org.dataloader.BatchLoaderEnvironment
+import org.easymock.EasyMock
+import org.testng.annotations.Test
+import java.util.UUID
+
+class BatchUserLoaderTest {
+    @Test
+    fun testUserLoader() = runBlocking {
+        val userDAO = LeakyMock.mock<UserDAO>()
+        val mockEnv = LeakyMock.mock<BatchLoaderEnvironment>()
+        val loader = BatchUserLoader(userDAO)
+        val users = listOf(
+            User(UUID.randomUUID(), UUID.randomUUID(), "joe", "[email protected]", "", false, ""),
+            User(UUID.randomUUID(), UUID.randomUUID(), "bill", "[email protected]", "", false, "")
+        )
+        EasyMock.expect(userDAO.get(EasyMock.anyObject<List<UUID>>() ?: listOf())).andReturn(users)
+        EasyMock.expect(mockEnv.getContext<Any?>()).andReturn(null)
+        EasyMock.replay(userDAO, mockEnv)
+        val us = loader.load(users.mapNotNull { it.id }.toSet(), mockEnv).await()
+        assertThat(us).isEqualTo(users.associateBy { it.id })
+        EasyMock.verify(userDAO, mockEnv)
+    }
+}
diff --git a/server/src/test/kotlin/com/joe/quizzy/server/graphql/dataloaders/BulkInstanceLoaderTest.kt b/server/src/test/kotlin/com/joe/quizzy/server/graphql/dataloaders/BulkInstanceLoaderTest.kt
@@ -0,0 +1,32 @@
+package com.joe.quizzy.server.graphql.dataloaders
+
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import com.joe.quizzy.api.models.Instance
+import com.joe.quizzy.persistence.api.InstanceDAO
+import com.trib3.testing.LeakyMock
+import kotlinx.coroutines.future.await
+import kotlinx.coroutines.runBlocking
+import org.dataloader.BatchLoaderEnvironment
+import org.easymock.EasyMock
+import org.testng.annotations.Test
+import java.util.UUID
+
+class BulkInstanceLoaderTest {
+    @Test
+    fun testInstanceLoader() = runBlocking {
+        val instanceDAO = LeakyMock.mock<InstanceDAO>()
+        val mockEnv = LeakyMock.mock<BatchLoaderEnvironment>()
+        val loader = BulkInstanceLoader(instanceDAO)
+        val instances = listOf(
+            Instance(UUID.randomUUID(), "i1", "ACTIVE", ""),
+            Instance(UUID.randomUUID(), "i2", "ACTIVE", "")
+        )
+        EasyMock.expect(instanceDAO.get(EasyMock.anyObject<List<UUID>>() ?: listOf())).andReturn(instances)
+        EasyMock.expect(mockEnv.getContext<Any?>()).andReturn(null)
+        EasyMock.replay(instanceDAO, mockEnv)
+        val insts = loader.load(instances.mapNotNull { it.id }.toSet(), mockEnv).await()
+        assertThat(insts).isEqualTo(instances.associateBy { it.id })
+        EasyMock.verify(instanceDAO, mockEnv)
+    }
+}