Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow construction of OffsetQueryPagingSource with Long #3409

Merged
merged 2 commits into from Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -629,14 +629,40 @@ class OffsetQueryPagingSourceTest {
assertTrue(pagingSource.jumpingSupported)
}

private fun query(limit: Int, offset: Int) = object : Query<TestItem>(
@Test
fun load_initialEmptyLoad_QueryPagingSourceLong() = runTest {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted a test to ensure that the constructor works as expected. This test is a copy of load_initialEmptyLoad, just using Long instead of Int.

val pagingSource = QueryPagingSourceLong(
countQueryLong(),
transacter,
EmptyCoroutineContext,
::queryLong,
)
val result = pagingSource.refresh() as LoadResult.Page

assertTrue(result.data.isEmpty())

// now add items
insertItems(ITEMS_LIST)

// invalidate pagingSource to imitate invalidation from running refreshVersionSync
pagingSource.invalidate()
assertTrue(pagingSource.invalid)

// this refresh should check pagingSource's invalid status, realize it is invalid, and
// return a LoadResult.Invalid
assertThat(pagingSource.refresh()).isInstanceOf(LoadResult.Invalid::class.java)
}

private fun query(limit: Int, offset: Int) = queryLong(limit.toLong(), offset.toLong())

private fun queryLong(limit: Long, offset: Long) = object : Query<TestItem>(
{ cursor ->
TestItem(cursor.getLong(0)!!)
},
) {
override fun <R> execute(mapper: (SqlCursor) -> R) = driver.executeQuery(1, "SELECT id FROM TestItem LIMIT ? OFFSET ?", mapper, 2) {
bindLong(0, limit.toLong())
bindLong(1, offset.toLong())
bindLong(0, limit)
bindLong(1, offset)
}

override fun addListener(listener: Listener) = driver.addListener(listener, arrayOf("TestItem"))
Expand All @@ -653,6 +679,16 @@ class OffsetQueryPagingSourceTest {
{ it.getLong(0)!!.toInt() },
)

private fun countQueryLong() = Query(
2,
arrayOf("TestItem"),
driver,
"Test.sq",
"count",
"SELECT count(*) FROM TestItem",
{ it.getLong(0)!! },
)

private fun insertItems(items: List<TestItem>) {
items.forEach {
driver.execute(0, "INSERT INTO TestItem (id) VALUES (?)", 1) {
Expand Down
Expand Up @@ -19,6 +19,7 @@ import androidx.paging.PagingConfig
import androidx.paging.PagingSource
import app.cash.sqldelight.Query
import app.cash.sqldelight.Transacter
import app.cash.sqldelight.db.SqlCursor
import kotlinx.coroutines.Dispatchers
import kotlin.coroutines.CoroutineContext
import kotlin.properties.Delegates
Expand Down Expand Up @@ -70,6 +71,35 @@ fun <RowType : Any> QueryPagingSource(
context,
)

/**
* Variant of [QueryPagingSource] that accepts a [Long] instead of an [Int] for [countQuery]
* and [queryProvider].
*
* If the result of [countQuery] exceeds [Int.MAX_VALUE], then the count will be truncated
* to the least significant 32 bits of this [Long] value.
*
* @see toInt
*/
@Suppress("FunctionName")
fun <RowType : Any> QueryPagingSourceLong(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer having this be the same name as the other function (QueryPagingSource) but we get a Platform declaration clash when doing so. Open to better names

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just switch them so the Int one has a fancy name since SQLDelight users will pretty much only use this one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using @JvmName should let you keep the names the same in Kotlin

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, just renamed it to QueryPagingSource after specifying @JvmName.

@AlecStrong I just suffixed the type to each one for consistency in the @JvmName. I'm assuming that no one's using SQLDelight with Java.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, perfect

countQuery: Query<Long>,
transacter: Transacter,
context: CoroutineContext = Dispatchers.IO,
queryProvider: (limit: Long, offset: Long) -> Query<RowType>,
): PagingSource<Int, RowType> = OffsetQueryPagingSource(
{ limit, offset -> queryProvider(limit.toLong(), offset.toLong()) },
countQuery.toInt(),
transacter,
context,
)

private fun Query<Long>.toInt(): Query<Int> =
object : Query<Int>({ cursor -> mapper(cursor).toInt() }) {
override fun <R> execute(mapper: (SqlCursor) -> R) = this@toInt.execute(mapper)
override fun addListener(listener: Listener) = this@toInt.addListener(listener)
override fun removeListener(listener: Listener) = this@toInt.removeListener(listener)
}

/**
* Create a [PagingSource] that pages through results according to queries generated by
* [queryProvider]. Queries returned by [queryProvider] should expected to do keyset paging.
Expand Down