6

I am developing an Android application using MVVM design pattern.

I have a class FCMService that extends FirebaseMessagingService.

As you may know, the FCMService overrides onMessageReceived(remoteMessage: RemoteMessage) function.

So, whenever I receive a message in onMessageReceived() function, I want to save it to room database via a repository.

It looks like the code below.

class FCMService : FirebaseMessagingService(), KodeinAware {

    override val kodein by closestKodein()
    private val repository: Repository by instance()
    private val scope: CoroutineScope by instance()

    override fun onNewToken(token: String) {
    }

    override fun onMessageReceived(remoteMessage: RemoteMessage) {
        super.onMessageReceived(remoteMessage)
        CoroutineScope(Dispatchers.IO).lauch{ repository.save(remoteMessage) }
    }
}
class Repository {
   suspend fun save(remoteMessage: RemoteMessage) {
      withContext(Dispatchers.IO) {
         someDAO.save(removeMessage)
      }
   }
}

I read a stackoverflow post and found out that the onMessageReceived() function executes in background thread and all work that is done within onMessageReceived(RemoteMessage message) should be done synchronously.

So, here are my questions please

  1. Should I use CoroutineScope(Dispatchers.IO).lauch {} in onMessageRecevied() function?

  2. If no, then I can just use normal function, not suspend function in repository and I can call it from onMessageReceived() without CoroutineScope(Dispatchers.IO).launch {}. Is it correct in terms of architectural design point of view please?

  3. It's a question about Coroutine but, as you can see that I launched a new coroutine in IO thread by CoroutineScope(Dispatchers.IO).lauch{ repository.save(remoteMessage) } in FCMService but I also switch the coroutineContext from IO to IO by withContext(Dispatchers.IO) { someDAO.save(removeMessage) } in Repository. I feel that withContext(Dispatchers.IO) { someDAO.save(removeMessage) } is unnecessary because I am switching from IO to IO. Am I right please?

KisungTae
  • 95
  • 1
  • 7

1 Answers1

8

I will try to answer to the best of my knowledge. Coming to your questions now.

Should I use CoroutineScope(Dispatchers.IO).lauch {} in onMessageRecevied() function?

I do not see any issue with it as such. Firebase Messaging Service is still a service basically, so there should be no problems with that. I would suggest you create a Coroutine scope that you can cancel though if anything goes wrong. Usually in ViewModel we use viewModelScope

So for that you can do something like this

val job = SupervisorJob()

 CoroutineScope(job).launch { 
   // Your Stuff here 
}

override fun onDestroy() {
    job.cancel()
    super.onDestroy()
}

Coming to your second question

If no, then I can just use normal function, not suspend function in repository and I can call it from onMessageReceived() without CoroutineScope(Dispatchers.IO).launch {}. Is it correct in terms of architectural design point of view please?

I would recommend you still use your Coroutine Scope instead of just directly using an normal function because it is recommend to use Room with Coroutines regardless and does you no harm even from an architecture standpoint.

Third

It's a question about Coroutine but, as you can see that I launched a new coroutine in IO thread by CoroutineScope(Dispatchers.IO).lauch{ repository.save(remoteMessage) } in FCMService but I also switch the coroutineContext from IO to IO by withContext(Dispatchers.IO) { someDAO.save(removeMessage) } in Repository. I feel that withContext(Dispatchers.IO) { someDAO.save(removeMessage) } is unnecessary because I am switching from IO to IO. Am I right please?

Since you are already using Dispatchers.IO you are correct that you do not need that again. Let it remain withContext(Dispatcher.IO) only to be consistent with your other structure.

che10
  • 2,176
  • 2
  • 4
  • 11
  • thank you for your response, so then I just need to create CoroutineScope in the service and get rid of ```withContext(Dispatchers.IO)``` in repository. Is that right? – KisungTae Jun 05 '21 at 22:29
  • @KisungTae Create a CouroutineScope in service to avoid any errors or leaks and do not get rid of `withContext(Dispatchers.IO)`. Let it stay the way it is. Instead just normally call it and inside the repository it will automatically shift to `IO` – che10 Jun 06 '21 at 04:58
  • I think onDestroy() method is not the best place for canceling a coroutine scope. The onDestroy() method can be called right after onMessageReceived().
 So your suspend call can be unfinished sometime. By official example you have ten seconds to process a message. 
So I think that it is better 1) to create a scope as a local variable 2) launch your coroutine 3) cancel it after ten seconds (by Handler). – Denis Arkharov Apr 23 '23 at 12:55