ServerManagerImpl is not thread safe

[TimoPtr]

Description

The current implementation of the server manager in the app ServerManagerImpl is not thread safe and has multiple potential race conditions.

• mutableServers is used to store temporary servers before (or not) adding them into the database

  • Unfortunately it means that the list needs to be in sync with the DB which is not always the case
  • If we call integrationRepository(ID) just after convertTemporaryServer most probably the mutableServers does not contains the server yet since it is updated asynchronously

• calling multiple times a function returning a repository might lead into the creation of multiple instances instead of one only. It is highly potential since we do inject the server manager in many places using different contexts

• updateServer might happen concurrently with something else touching the same server

Ideas

• We could create a new addServer function that is transactional using DSL something like

    class AddServerDsl(
        val authenticationRepository: AuthenticationRepository,
        val integrationRepository: IntegrationRepository,
    ) {

        var commit: Boolean = false
    }

    suspend fun addServer(server: Server, dsl: suspend AddServerDsl.() -> Unit) {
        val tempServerId: Int = -2 // createTempServer()
        val addServerDsl =
            AddServerDsl(
                authenticationRepositoryFactory.create(tempServerId),
                integrationRepositoryFactory.create(tempServerId),
            )
        addServerDsl.dsl()
        // check the status
        if (addServerDsl.commit) {
            // Add to the database
            val server = serverDao.add(server.copy(id = 0))
        } else {
            // Server not added
        }
    }

    // ...

    suspend fun foo(
        code: String,
        deviceName: String,
        appVersionProvider: AppVersionProvider,
        messagingTokenProvider: MessagingTokenProvider,
    ) {
        val server = Server(
            _name = "",
            type = ServerType.TEMPORARY,
            connection = ServerConnectionInfo(
                externalUrl = "http://ha.org",
            ),
            session = ServerSessionInfo(),
            user = ServerUserInfo(),
        )

        addServer(server) {
            authenticationRepository.registerAuthorizationCode(code)
            integrationRepository.registerDevice(
                DeviceRegistration(
                    appVersionProvider(),
                    deviceName,
                    messagingTokenProvider(),
                ),
            )
            commit = true
        }
    }

• We might only keep the DB as source of truth and remove the mutableServers. In any case we should most probably start using a Mutex to make all the methods thread safe