twilio / twilio-chat-demo-android

Chat API Demo Application for Android
MIT License
62 stars 51 forks source link

Code Improvement #145

Closed rooparsh closed 4 years ago

rooparsh commented 4 years ago

Improving code quality for fetching messages in a channel

When we fetch data from any channel , some of them are media messages. Right now with the latest sdk , we have to call the getContentTemporaryUrl method for each media message then display the image/video from url via glide/coil. The current code that we are using , it waits for all the urls to get fetched and then we publish the list to recycler view.

Code (optional)

  private suspend fun handleMessages(listOfMessages: List<Message>?) {
    listOfMessages?.let { list ->
      if (list.isNotEmpty()) {
        val messageList = computation {
          oldestMessageIndex = list[0].messageIndex
          list.map {
            if (it.hasMedia()) {
              addMediaMessage(it)
            } else {
              addNonMediaMessage(it)
            }
          }
            .asReversed()
        }

        _viewState.postValue(ViewState.Success(null))
        _messages.postValue(messageList)
      } else {
        _viewState.postValue(ViewState.Error(null))
      }
    } ?: kotlin.run { _viewState.postValue(ViewState.Error(null)) }
  }
 private suspend fun addMediaMessage(it: Message): com.example.chat.model.Message {
    return when {
      it.media.type.contains("image") -> addImageMessageToList(it)
      it.media.type.contains("video") -> addVideoMessageToList(it)
      it.media.type.contains("audio") -> addAudioMessageToList(it)
      it.media.type.contains("application/pdf") -> addFileMessageToList(it)
      else -> addNotSupportedMessageToList(it)
    }
  }
  private suspend fun addImageMessageToList(it: Message): com.example.chat.model.Message {

    val imageUrl = io {
      fetchMediaUrlUseCase.perform(it.media)
    }
    return when (imageUrl) {
      is Success -> {
        ImageMessage(imageUrl.body ?: "").apply {
          setSender(it.author)
          setMessageId(it.media.sid)
          setTimestamp(it.dateCreated.getTimeInMillis())
        }
      }
      else -> {
        TextMessage("failed to load").apply {
          setSender(it.author)
          setMessageId(it.sid)
          setTimestamp((it.dateCreated.getTimeInMillis()))
        }
      }
    }
  }

Can you suggest something to improve performance

Chat Android SDK 6.0.0

rusmonster commented 4 years ago

I'd suggest put a stub image into view and show it immediately, at the same time make async request which fetch imageUrl using sdk then download the image then update the view.

rooparsh commented 4 years ago

@rusmonster Even I thought the same. But I wanted the UI to be unaware of the Twilio. So how do you suggest I make an API call? Are you suggesting interface method from onBindViewHolder?

rooparsh commented 4 years ago

@rusmonster Is there any way I can fetch media Url from just message sid? Right now, my viewHolder has just message sid in model. I am thinking of maybe fetch urls in recyclerView oBindViewHolder using sid?

rusmonster commented 4 years ago

In term of your approach I'd suggest smth like:

 private fun addImageMessageToList(it: Message): com.frameshift.chat.model.Message {
      val imageMessage = ImageMessage(imageUrl = STUB_IMAGE_URL).apply {
          setSender(it.author)
          setMessageId(it.media.sid)
          setTimestamp(it.dateCreated.getTimeInMillis())
      }
      fetchMediaUrlAsync(it, imageMessage)
      return imageMessage;
  }

 private fun fetchMediaUrlAsync(message: Message, model: com.frameshift.chat.model.Message) = viewModelScope.launch {    
    val imageUrl = io { fetchMediaUrlUseCase.perform(message.media) }

    when (imageUrl) {
      is Success -> model.imageUrl.postValue(imageUrl.body) // model.imageUrl is MutableLiveData<Uri>

      else -> model.imageUrl.postValue(LOAD_ERROR_IMAGE_URL)
    }
 }

class ChannelListAdapter(
    private val owner: LifecycleOwner // pass activity here
): RecyclerView.Adapter<...>() {

    var messages: List<com.frameshift.chat.model.Message> by Delegates.observable(emptyList()) { _, old, new ->
        new.forEachIndexed { index, message ->
            message.imageUrl.observe(owner, Observer<Uri> {
                notifyItemChanged(index)
            })
        }
    }

    override fun onBindViewHolder(holder: MessageViewHolder, position: Int) {
        val message = messages[position]
        GlideApp.with(this)
            .load(message.imageUrl.value)
            .into(holder.imageView). 
    }
}

In general if it's not too late I'd suggest to switch to the Recommended app architecture where most part of such questions solved out from the box. BTW we are going to publish new twilio demo apps later this year which will demonstrate how to build an app with twilio chat sdk using that architecture.