stream-chat-android
stream-chat-android copied to clipboard
[ISSUE-4109] Using Repositories implementations instead of facade
🎯 Goal
Using implementation of repositories instead of RepositoryFacade
.
🧪 Testing
Test the persistence of the SDK
☑️Contributor Checklist
General
- [x] I have signed the Stream CLA (required)
- [x] Assigned a person / code owner group (required)
- [ ] Thread with the PR link started in a respective Slack channel (#android-chat-core or #android-chat-ui) (required)
- [x] PR targets the
develop
branch - [x] PR is linked to the GitHub issue it resolves
Code & documentation
- ~[ ] Changelog is updated with client-facing changes~
- ~[ ] New code is covered by unit tests~
- ~[ ] Comparison screenshots added for visual changes~
- ~[ ] Affected documentation updated (KDocs, docusaurus, tutorial)~
☑️Reviewer Checklist
- [ ] UI Components sample runs & works
- [ ] Compose sample runs & works
- [ ] UI Changes correct (before & after images)
- [ ] Bugs validated (bugfixes)
- [ ] New feature tested and works
- [ ] Release notes and docs clearly describe changes
- [ ] All code we touched has new or updated KDocs
🎉 GIF
A facade
Hi @JcMinarro.
One problem that we have with the facade is that we use some overrides in the original implementations that cause duplication when using the interfaces. For example:
userRepository.insertUsers(messageToBeDeleted.users())
messageRepository.insertMessage(messageToBeDeleted, true)
This code would call insert user twice because of the implementation of insertMessage of RepositoryFacade:
override suspend fun insertMessage(message: Message, cache: Boolean) {
insertUsers(message.users())
messageRepository.insertMessage(message, cache)
}
Using RepositoryFacade
brings this problem with it. Another solution, instead of using the implementations inside the facade would be to erase this method and make sure the other classes are not having this problem (pehaps clean all override), but it is a bigger change.
What do you think?
Hi @JcMinarro.
One problem that we have with the facade is that we use some overrides in the original implementations that cause duplication when using the interfaces. For example:
userRepository.insertUsers(messageToBeDeleted.users()) messageRepository.insertMessage(messageToBeDeleted, true)
This code would call insert user twice because of the implementation of insertMessage of RepositoryFacade:
override suspend fun insertMessage(message: Message, cache: Boolean) { insertUsers(message.users()) messageRepository.insertMessage(message, cache) }
Using
RepositoryFacade
brings this problem with it. Another solution, instead of using the implementations inside the facade would be to erase this method and make sure the other classes are not having this problem (pehaps clean all override), but it is a bigger change.What do you think?
Just that is the case. Whenever a channel/message is added, we need to add all related users with this entity. If we forgot to insert them in some place, our SDK could crash when the entity is recovered (And indeed, it was crashing in the past until we implemented the facade and handled this case)
If someone inserts twice the same user, it doesn't create a duplicate. The code you posted is totally fine.
Hey @JcMinarro 👋
The main issue is that we're doing unnecessary IO
operations, at least 2 times more than it actually requires 🤷♂️
So I suppose by duplicate
@leandroBorgesFerreira meant not the entities but IO
operations.
Hey @JcMinarro wave
The main issue is that we're doing unnecessary
IO
operations, at least 2 times more than it actually requires man_shrugging So I suppose byduplicate
@leandroBorgesFerreira meant not the entities butIO
operations.
Those unnecessary IO
operations can be avoided by removing the insertUsers()
call from outside of the repositoryFacade
invocation, because they are not needed.
But we can't allow other users to insert corrupt data to our DB