From 6d5e0973b4b7140d7658017aab0098d0ebdf41b0 Mon Sep 17 00:00:00 2001
From: Truls <truls.stenrud@gmail.com>
Date: Tue, 29 Aug 2023 10:55:28 +0200
Subject: [PATCH] Use offset and limit in sql if domainFIlter is null

---
 .../kotlin/no/liflig/documentstore/dao/Dao.kt | 30 +++++++++++------
 .../documentstore/SearchRepositoryTest.kt     | 33 +++++++++++++++++--
 .../documentstore/examples/ExampleQuery.kt    |  2 +-
 3 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/src/main/kotlin/no/liflig/documentstore/dao/Dao.kt b/src/main/kotlin/no/liflig/documentstore/dao/Dao.kt
index 343ddb6..16b2aaa 100644
--- a/src/main/kotlin/no/liflig/documentstore/dao/Dao.kt
+++ b/src/main/kotlin/no/liflig/documentstore/dao/Dao.kt
@@ -15,7 +15,6 @@ import java.io.InterruptedIOException
 import java.sql.SQLTransientException
 import java.time.Instant
 import java.util.UUID
-import kotlin.streams.asSequence
 
 // TODO: update docs
 /**
@@ -255,7 +254,7 @@ abstract class AbstractSearchRepository<I, A, Q>(
     offset: Int? = null,
     orderBy: String? = null,
     orderDesc: Boolean = false,
-    domainFilter: ((A) -> Boolean) = { true },
+    domainFilter: ((A) -> Boolean)? = null,
     bind: Query.() -> Query = { this }
   ): List<VersionedEntity<A>> = mapExceptions {
     val transaction = transactionHandle.get()
@@ -276,29 +275,40 @@ abstract class AbstractSearchRepository<I, A, Q>(
     offset: Int? = null,
     orderBy: String? = null,
     desc: Boolean = false,
-    domainFilter: ((A) -> Boolean) = { true },
+    domainFilter: ((A) -> Boolean)? = null,
     bind: Query.() -> Query = { this }
   ): List<VersionedEntity<A>> {
 
     val orderDirection = if (desc) "DESC" else "ASC"
     val orderByString = orderBy ?: "created_at"
 
-    return handle
+    // only use if domainFilter is null
+    val limitString = limit?.let { "LIMIT $it" }?.takeIf { domainFilter == null } ?: ""
+    val offsetString = offset?.let { "OFFSET $it" }?.takeIf { domainFilter == null } ?: ""
+
+    val result = handle
       .select(
         """
               SELECT id, data, version, created_at, modified_at
               FROM "$sqlTableName"
               WHERE ($sqlWhere)
               ORDER BY $orderByString $orderDirection
+              $limitString
+              $offsetString
         """.trimIndent()
       )
       .bind()
       .map(rowMapper)
-      .asSequence()
-      .filter { domainFilter(it.item) }
-      .run { offset?.let { drop(it) } ?: this }
-      .run { limit?.let { take(it) } ?: this }
-      .toList()
+
+    return if (domainFilter == null)
+      result.list()
+    else {
+      result.asSequence()
+        .filter { domainFilter(it.item) }
+        .run { offset?.let { drop(it) } ?: this }
+        .run { limit?.let { take(it) } ?: this }
+        .toList()
+    }
   }
 }
 
@@ -309,7 +319,7 @@ abstract class QueryObject<A> {
   open val offset: Int? = null
   open val orderBy: String? = null
   open val orderDesc: Boolean = false
-  open val domainFilter: ((A) -> Boolean) = { true }
+  open val domainFilter: ((A) -> Boolean)? = null
 }
 
 class SearchRepositoryJdbi<I, A, Q>(
diff --git a/src/test/kotlin/no/liflig/documentstore/SearchRepositoryTest.kt b/src/test/kotlin/no/liflig/documentstore/SearchRepositoryTest.kt
index ca7f061..f037fad 100644
--- a/src/test/kotlin/no/liflig/documentstore/SearchRepositoryTest.kt
+++ b/src/test/kotlin/no/liflig/documentstore/SearchRepositoryTest.kt
@@ -91,6 +91,32 @@ class SearchRepositoryTest {
     }
   }
 
+  @Test
+  fun `offset and limit works with domain filter`() {
+    runBlocking {
+      dao.create(createEntity("A"))
+      dao.create(createEntity("B"))
+      dao.create(createEntity("C"))
+      dao.create(createEntity("D"))
+      dao.create(createEntity("this-A"))
+      dao.create(createEntity("this-B"))
+      dao.create(createEntity("this-C"))
+      dao.create(createEntity("this-D"))
+
+      val result =
+        searchRepository.search(
+          ExampleQuery(
+            offset = 1,
+            limit = 2,
+            domainFilter = { it.text.startsWith("this") })
+        )
+          .map { it.item.text }
+
+      result shouldHaveSize 2
+      result shouldBeEqual listOf("this-B", "this-C")
+    }
+  }
+
   @Test
   fun `empty search returns all items`() {
     runBlocking {
@@ -111,11 +137,13 @@ class SearchRepositoryTest {
       dao.create(createEntity("B"))
       dao.create(createEntity("C"))
 
-      val result = searchRepository.search(ExampleQuery(orderBy = "data->>'text'", orderDesc = false)).map { it.item.text }
+      val result =
+        searchRepository.search(ExampleQuery(orderBy = "data->>'text'", orderDesc = false)).map { it.item.text }
 
       result shouldBeEqual listOf("A", "B", "C")
     }
   }
+
   @Test
   fun `orderDesc flips direction`() {
     runBlocking {
@@ -123,7 +151,8 @@ class SearchRepositoryTest {
       dao.create(createEntity("B"))
       dao.create(createEntity("C"))
 
-      val result = searchRepository.search(ExampleQuery(orderBy = "data->>'text'", orderDesc = false)).map { it.item.id }
+      val result =
+        searchRepository.search(ExampleQuery(orderBy = "data->>'text'", orderDesc = false)).map { it.item.id }
       val resul2 = searchRepository.search(ExampleQuery(orderBy = "data->>'text'", orderDesc = true)).map { it.item.id }
 
       result shouldBeEqual resul2.asReversed()
diff --git a/src/test/kotlin/no/liflig/documentstore/examples/ExampleQuery.kt b/src/test/kotlin/no/liflig/documentstore/examples/ExampleQuery.kt
index 51947da..5e58617 100644
--- a/src/test/kotlin/no/liflig/documentstore/examples/ExampleQuery.kt
+++ b/src/test/kotlin/no/liflig/documentstore/examples/ExampleQuery.kt
@@ -8,7 +8,7 @@ class ExampleQuery(
   override val offset: Int? = null,
   override val orderBy: String? = null,
   override val orderDesc: Boolean = false,
-  override val domainFilter: ((ExampleEntity) -> Boolean) = { true },
+  override val domainFilter: ((ExampleEntity) -> Boolean)? = null,
 
   val text: String? = null,