Search code examples
androidkotlinktorkotlin-exposed

Generic service for CRUD requests in Ktor


I've generated a new Ktor project in which I could find a UsersSchema.kt file.

@Serializable
data class ExposedUser(val name: String, val age: Int)

class UserService(database: Database) {
    object Users : Table() {
        val id = integer("id").autoIncrement()
        val name = varchar("name", length = 50)
        val age = integer("age")

        override val primaryKey = PrimaryKey(id)
    }

    init {
        transaction(database) {
            SchemaUtils.create(Users)
        }
    }

    suspend fun create(user: ExposedUser): Int = dbQuery {
        Users.insert {
            it[name] = user.name
            it[age] = user.age
        }[Users.id]
    }

    suspend fun read(id: Int): ExposedUser? {
        return dbQuery {
            Users.selectAll()
                .where { Users.id eq id }
                .map { ExposedUser(it[Users.name], it[Users.age]) }
                .singleOrNull()
        }
    }

    suspend fun update(id: Int, user: ExposedUser) {
        dbQuery {
            Users.update({ Users.id eq id }) {
                it[name] = user.name
                it[age] = user.age
            }
        }
    }

    suspend fun delete(id: Int) {
        dbQuery {
            Users.deleteWhere { Users.id.eq(id) }
        }
    }

    private suspend fun <T> dbQuery(block: suspend () -> T): T =
        newSuspendedTransaction(Dispatchers.IO) { block() }
}

Based on this example, I created a CategorySchema.kt file.

@Serializable
data class Category(
    val id: String,
    val name: String,
    val image: String?,
    val slug: String,
)

class CategoryService(database: Database) {
    object CategoryTable: Table("\"Category\"") {
        val id = varchar("id", 36).uniqueIndex()
        val name = varchar("name", 50).uniqueIndex()
        val image = text("image").nullable()
        val slug = varchar("slug", 50).uniqueIndex()
        val createdAt = datetime("createdAt")
        val updatedAt = datetime("updatedAt")

        override val primaryKey = PrimaryKey(id)
    }

    init {
        transaction(database) {
            SchemaUtils.create(CategoryTable)
            addLogger(StdOutSqlLogger)
        }
    }

    suspend fun getAll(): List<Category> {
        try {
            return dbQuery {
                CategoryTable.selectAll().map {
                    Category(
                        it[CategoryTable.id],
                        it[CategoryTable.name],
                        it[CategoryTable.image],
                        it[CategoryTable.slug]
                    )
                }
            }
        } catch (e: Exception) {
            exposedLogger.error("Aucune catégorie existante")
            return emptyList()
        }
    }

    suspend fun getOne(id: String): Category? {
        try {
            return dbQuery {
                CategoryTable
                    .selectAll()
                    .where { CategoryTable.id eq id }
                    .map {
                        Category(
                            it[CategoryTable.id],
                            it[CategoryTable.name],
                            it[CategoryTable.image],
                            it[CategoryTable.slug]
                        )
                    }
                    .singleOrNull()
            }
        } catch (e: Exception) {
            exposedLogger.error("La catégorie recherchée n'existe pas")
            return null
        }
    }

    private suspend fun <T> dbQuery(block: suspend () -> T): T =
        newSuspendedTransaction(Dispatchers.IO) { block() }
}

Then I realised that many of my tables have similar methods, and as I didn't want to repeat myself, I decided to create a BaseService class.

abstract class BaseService<Entity, ExposedTable : Table>(
    private val table: ExposedTable,
    private val mapper: (ResultRow) -> Entity
) {
    suspend fun getAll(): List<Entity> {
        try {
            return dbQuery {
                table.selectAll().map(mapper)
            }
        } catch (e: Exception) {
            exposedLogger.error("Erreur lors de la récupération des éléments issus de ${table.tableName}")
            return emptyList()
        }
    }
    
    private suspend fun <T> dbQuery(block: suspend () -> T): T =
        newSuspendedTransaction(Dispatchers.IO) { block() }
}

However, I don't know how I should write the getOne() method.

What I've tried:

suspend fun getOne(id: String): Entity? {
    try {
        return dbQuery {
            table
                .selectAll()
                .where { table.primaryKey eq id }
                .map(mapper)
                .singleOrNull()
        }
    } catch (e: Exception) {
        exposedLogger.error("Erreur lors de la récupération de l'élément issu de ${table.tableName}")
        return null
    }
}

Error:

Type mismatch. Required: Op Found: Unit

suspend fun getOne(id: String): Entity? {
    val key = table.primaryKey?.columns?.singleOrNull() ?: throw IllegalStateException("Error")
    try {
        return dbQuery {
            table
                .select { key eq id }
                .map(mapper)
                .singleOrNull()
        }

Error:

Type mismatch. Required: List<Expression<*>> Found: () → Unit

What I want to do:

I would like to create an API with only GET and POST methods. I should be able to get data from all my tables and update some of them. As the GET methods always work in the same way, I wanted to create a generic service with two methods getAll() and getOne(). Thanks to this, I could pass them parameters and use them in all my services.

More informations:

UsersSchema.kt was generated by Ktor: I used it as an example and then removed it from my project. I generated my database tables with Prisma and they all have an id property which is a String (a UUID to be more precise, but I'll handle that later). So, if a table doesn't have an id property of type String, it means that there's an error in the generation of the table (which isn't part of this project). In fact, all OtherTable will be like CategoryTable, i.e. an object with a String id and other values that are not necessarily common to all tables.


Solution

  • You assume the primary key to always be a single column of type String. That is true for CategoryTable, but for Users it is an Int. It can potentially be anything, so getOne(id: String) will be wrong in most cases.

    Even worse, a primary key can span multiple columns. You only use a single column for CategoryTable and Users, but this is not true in general.

    You could do something like this:

    val key = table.primaryKey?.columns?.first() as Column<String>
    

    And then you use

    .where { key eq id }
    

    in your getOne(id: String) function, but that will result in a runtime exception when the first part of the primary key won't be a String.

    You could try to make the type of the primary key part of your generic BaseService, but then again, a table is not even required to have a primary key, table.primaryKey could be null.


    Instead I would challenge why you even want to create a generic BaseService in the first place.

    The only thing that you do in getAll is to call selectAll() and apply the mapper, which is pretty trivial.

    But you also wrap everything in an error handler which swallows all exceptions. That is actually a bad thing: The caller does not even get notified that something went wrong (and whatever went wrong when calling table.selectAll().map(mapper), it is most certainly an error to just ignore it and return an empty list). Even worse, you also swallow any CancellationExceptions that may occur. In suspend functions you must always make sure that all CancellationExceptions are rethrown (or not even caught in the first place), otherwise you will break the coroutine cancellatioin mechanism.

    I would strongly recommend to handle errors at some place later where they can be more meaningfully handled.

    Together with the experience with getOne that looks simple but is quite complex to get right, the generic approach seems to be too unflexible to be of any use.

    In my opinion you should simply keep it as it is. As you already figured out, you can extract the logic to create the table, as well as the mapping logic of a ResultRow. I would not do anything more to the UserService and CategoryService.