Repository layer for MongoDB

Question

I've implemented a quick and dirty repository to save and insert users to a MongoDB. As you can see, it just inserts one and finds one. Not a rocket.

The thing is I don't really know what are the best practices when implementing a repository layer, how to really arquitect this, etc...

Furthermore, even if it sounds too simple, I'm not sure which variables should be instance properties or not (for example, the database, or the client). Assumed those where convenient, but I'm not sure on the choice.

Any tips are welcome :)

User.kt

data class User (val chatId: String, val username: String, val email: String)

BaseRepository.kt

import com.mongodb.MongoClient
import com.mongodb.MongoClientURI
import com.mongodb.client.MongoDatabase

abstract class BaseRepository<Entity>(databaseUri: String) {
    protected var database: MongoDatabase? = null
    protected var mongoClient: MongoClient? = null

    init {
        val connectionString = MongoClientURI(databaseUri)
        mongoClient = MongoClient(connectionString)
        database = mongoClient?.getDatabase("paybotDB")
    }

    abstract fun insertOne(e: Entity);

    abstract fun findOne(username: String) : Entity;
}

UserRepository.kt

import org.bson.Document
import org.jgmanzano.paybot.entities.User

class UserRepository(databaseUri: String) : BaseRepository<User>(databaseUri) {

    override fun insertOne(user: User){
        val document = Document("username", user.username)
                .append("chatId", user.chatId)
                .append("email", user.email)

        database?.getCollection("users")?.insertOne(document)
    }

    override fun findOne(username: String): User {
        val collection = database?.getCollection("users")
        val userDocument =  collection?.find(Document("username", username))?.first()

        return User(
                userDocument?.get("chatId") as String,
                userDocument?.get("username") as String,
                userDocument?.get("email") as String
        )
    }
}

Answer 1

Your properties are nullable even tough they are never null and assigned in the constructor. Declare them as

protected val database: MongoDatabase

This way, the child classes can access them without needing the safe-call operator and also you prevent them from overwriting the property.

Also, both of your methods call database.getCollection("users"). This is a sign that you need to extract the collection to a property rather than the database or client.

Also, it's better to pass a MongoClient or even a MongoDatabase to the constructor because it can be shared between multiple repositories and follows the dependency injection pattern.

Because you now have only one property, you can drop the init block.

abstract class BaseRepository<Entity>(db: MongoDatabase, collectionName: String) {
    protected val collection = db.getCollection(collectionName)

    abstract fun insertOne(e: Entity)

    abstract fun findOne(username: String) : Entity
}

I also got rid of the unnecessary semicolons.

Now to the child class. val userDocument = collection?.find(Document("username", username))?.first() returns a nullable Document?. But in the line below you cast the members to String which will throw an exception if the value was null. To get proper null safety, declare the return type of the method as nullable

abstract fun findOne(username: String) : Entity?

and rewrite the implementation, e.g. like so

override fun findOne(username: String): User? {
    val userDocument = collection.find(Document("username", username)).first() ?: return null

    return User(
            userDocument.get("chatId") as String,
            userDocument.get("username") as String,
            userDocument.get("email") as String
    )
}