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

Custom getters with Global Database object removed #73

Closed
ashokgelal opened this issue Jan 5, 2020 · 8 comments
Closed

Custom getters with Global Database object removed #73

ashokgelal opened this issue Jan 5, 2020 · 8 comments

Comments

@ashokgelal
Copy link

ashokgelal commented Jan 5, 2020

I've been trying Ktorm 2.7-RC that you uploaded after my request (thank you!).

In practice, I've found the new way of using Ktorm quite uncomfortable as now I've to pass around a database object. While this is doable, one place where I've been struggling with the new APIs is the custom setters. A while back you gave us an example of using a custom getter inside an entity interface:

interface Department : Entity<Department> {
    val id: Int
    val name: String
    val employees get() = Employees.findList { it.departmentId eq id }
}

Do you have an idea how to achieve this with the new API?

@vincentlauvlwj
Copy link
Member

If you find it's a problem passing the database object around, you can define it as a top-level variable:

val globalDatabase = Database.connect(..)

You can even write an extension function getList that uses this globalDatabase and obtains a list of entities that matches the given condition:

inline fun <E : Any, T : BaseTable<E>> T.getList(predicate: (T) -> ColumnDeclaring<Boolean>): List<E> {
    return globalDatabase.sequenceOf(this).filter(predicate).toList()
}

Then just like before:

interface Department : Entity<Department> {
    val id: Int
    val name: String
    val employees get() = Employees.getList { it.departmentId eq id }
}

@ashokgelal
Copy link
Author

Unfortunately, the top level global variable would not work for me.

On a related note, I was checking out Exposed code base to see how they are doing it without passing a global DB object around as well as supporting nested transactions, and looks like they are doing very similar to what you were doing (using a transaction block) before 2.7. Have you considered doing something similar or it wouldn't just work for Ktorm? Any thoughts?

@vincentlauvlwj
Copy link
Member

Unfortunately, the top level global variable would not work for me.

Before 2.7, Ktorm saved a global database object to Database.global. After 2.7, we just need to define a top-level variable (if needed) to save the global database object by ourselves. They are both global objects, so what is the real difference between them? I don't know why the top-level global variable doesn't work for you.

On a related note, I was checking out Exposed code base to see how they are doing it without passing a global DB object around as well as supporting nested transactions, and looks like they are doing very similar to what you were doing (using a transaction block) before 2.7. Have you considered doing something similar or it wouldn't just work for Ktorm? Any thoughts?

I know Exposed. I read its code before. Actually, the design of Database.global was made inspired by Exposed. But now, I've realized the disadvantages of using global objects and thread locals, that's why I decided to deprecate it.

@vincentlauvlwj
Copy link
Member

vincentlauvlwj commented Jan 5, 2020

I found your code OzoneProvider. Maybe in this class you connect to the database. Just need a little change:

lateinit var globalDatabase: Database

class OzoneProvider : ServiceProvider {
    override fun register(app: Application) {
        val dbConfig = app.config { DatabaseConfig(it.make()) }
        if (dbConfig.canConnect()) {
            globalDatabase = dbConfig.connect()
        }
    }
}

Would it work now?

@ashokgelal
Copy link
Author

I actually thought about doing that but I'd have to make it a lateinit var making it mutable from anywhere. Maybe I leave it like that or do something so that it can only be initialized once.

@ashokgelal
Copy link
Author

You can even write an extension function getList that uses this globalDatabase and obtains a list of entities that matches the given condition:

inline fun <E : Any, T : BaseTable<E>> T.getList(predicate: (T) -> ColumnDeclaring<Boolean>): List<E> {
    return globalDatabase.sequenceOf(this).filter(predicate).toList()
}

Yes, but then getList depends on the global db connection which means I wouldn't have any control over running it on a different connection. Severely restricts the power of connection switching and, on a initial thought, in a way defeats the purpose of removing the global db object.

@vincentlauvlwj
Copy link
Member

I actually thought about doing that but I'd have to make it a lateinit var making it mutable from anywhere. Maybe I leave it like that or do something so that it can only be initialized once.

You can change the visibility of the setter, so that it can only be modified inside the kt file:

lateinit var globalDatabase: Database 
    private set

@vincentlauvlwj
Copy link
Member

Yes, but then getList depends on the global db connection which means I wouldn't have any control over running it on a different connection. Severely restricts the power of connection switching and, on a initial thought, in a way defeats the purpose of removing the global db object.

Yes, that's exactly why I decided to remove Database.global. Here I just provide a workaround for you because you might want to use the global database like before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants