copper-leaf
Refactor Repository module
After some time using and thinking about the Repository module, I've come to the conclusion that it's current stated use-case is not the most ideal for structuring your application. Most notably, the method of using an EventBus for communication between Repository instances is flawed. Instead, Repositories that depend on the state of other Repositories should be arranged in a hierarchy (either a true tree, or a Directed Acyclic Graph (DAG)), rather than something like an unstructured Graph as is the current setup.
The inspiration for this change comes from thinking about the application as a whole, and understanding the purpose of the Repository layer and the Repositories within that. Just as the UI screens and their ViewModels only ever passively observe from Repositories, and thereby isolate themselves from the implementation details of the Repository while also eliminating strange communication patterns by enforcing the UDF flow, the same should be true of Repositories amongst themselves. Parent repositories should send their state to children, which eventually flows to UIs, to deeper components within the UI, etc.
Ultimately, it should follow a flow that is something like this:
flowchart TD
Global["Global State (authentication)"]
Router
Repositories
Screens
Components
Global --> Router
Global --> Repositories
Router --> Screens
Repositories --> Screens
Screens --> Components
The extent of changes should be as follows:
- Remove the Repository base classes. Just use the standard ViewModels already available. Each Repository should define its own Events as with any other ViewModel (though there may not be many use-cases for them in Repositories)
- Make the
CachedAPI generic enough such that it could be used in any ViewModel, or even outside of a ViewModel if it needs to be done in a standard suspending function - Create an out-of-the-box implementation for the Contract and InputHandler that makes it very easy to plug the
Cachedwrapper into a Repository ViewModel without all the boilerplate required now - Remove the
EventBus - Rewrite documentation to encourage passing parent ViewModels into the constructor of another, rather than reading from an EventBus
Additional nice-to-have features include:
- Support additional methods of busting the
Cachedvalues in the Repository, such as time-based or (depending on platform) based on total size of data stored in the ViewModel
Found this issue linked from Slack.
Thank you for this. I found when getting started with Ballast the repository concept to be among the most difficult to understand and create the "correct" architecture for. I believe these changes will go a long way to improving that.
When a repository is "injected" into another via constructor, would the "parent" repository simply call public methods of the injected repository? Or would there be some kind of intermediate layer provided by Ballast that helps manage and control that communication?
The idea would be to only use the public methods of one repository in another, and passing one repo into another via constructors enforces a hierarchy among them. Mostly, it would be a rewrite of the documentation, with some additional utilities to help with specific features commonly used in repositories like caching API calls or observing flows from a DB.
The idea would be to only use the public methods of one repository in another, and passing one repo into another via constructors enforces a hierarchy among them.
This is what we found worked best on our project as well. Great!
- Rewrite documentation to encourage passing parent ViewModels into the constructor of another
View models at the UI layers usually depend on some local context, most importantly some lifecycle information e.g. coroutine scope.
Therefore, presumably these lower layer view models won't be passed around in constructors.
Only view models that are instantiated at app startup and live for the entire time the application is running would be passed into constructors right? Will there be a simplified way to create view models such as these, such that they can be created in a DI system easily?
Also, when communicating state from parents to children in response to children calling their parents, it would be great to have a canonical way to solve the problem I mentioned to you in some Slack DMs a while back.
Using CompletableDeferred was one option we had discussed, and works but is ugly. I would have preferred passing lambdas that update the local state instead, which I think is a reasonable way for a parent VM to update the state in a child VM (similar to how a child UI component might update state in a parent). However, this approach does not work well due to a state exception when the lambda is executed:
IllegalStateException: This InputHandlerScope has already been closed
When thinking about refactoring the repository module, is it worth considering some built-in way for a parent repo to communicate state back to a child in response to a call on the parent?
Therefore, presumably these lower layer view models won't be passed around in constructors.
That's correct, UI ViewModels should never depend on one another, they can only depend on the Repositories. It's only the "Repository ViewModels" (or just "Repositories") that depend on each other, and when they do, it's in a strict hierarchy enforced by constructor injection.
Will there be a simplified way to create view models such as these...
I haven't thought this through entirely yet, but the main thing I've got in my head (and what I'm doing in my projects right now) is to just use a normal BasicViewModel, where the CoroutineScope is injected internally from within DI rather than passed into a factory DI function.
communicating state from parents to children
This is the main thing I'm still thinking through, both for how to easily manage a lot of state values in the Repository, and how to easily share those with the children dependencies. I don't think it should stray far from the current model for consuming it (parent state will still just be exposed as a StateFlow), but there is definitely opportunity to simplify this whole process, and I've got a few ideas that might work.
And yes, this would need to cover all use-cases of:
- The Repo observing a reactive data source and emitting a Flow
- The Repo making a one-shot query internally and caching the result
- A child requesting a one-shot change and the Repository returning a result
is it worth considering some built-in way for a parent repo to communicate state back to a child
It is absolutely worth considering, and I've got some ideas on how it could be implemented (for example, have the child State implement an interface that allows it to be updated directly from an Interceptor). I'll play around with the idea, but I'm not sure yet if it will be something that would be generalizable enough to include in the library, or just be a recommendation that you would handle yourself.
@cjbrooks12 I'm thinking about using Ballast on a new project, and I'm circling back around to discussions like this, as well as our past Slack DMs, to refresh my memory. Have you given any further thought to concretizing the discussions here into some doc updates and and utility functions in Ballast?
I think where I've landed on the architectural issue is that Ballast works best when limited to the UI/VM layer, and you'll have better results if you leverage other libraries or patterns for the overall app architecture. In particular, Store is probably the most applicable library replacement for what I initially envisioned the Ballast Repository module to accomplish.
However, in my own apps I have found it more effective overall to not use libraries at all for building out the app's business logic, but instead to closely follow the architecture outlined in the Android documentation. And specifically, most apps I've worked on had separate Data and UI layers, but introducing an explicit Domain layer with Use Cases that handle the business logic gives the flexibility and maintainability that is needed in large enterprise-scale apps.
In a nutshell, my app's architectures look like this:
- Data layer: stateless wrappers around data sources like REST APIs, local DB, SharedPreferences, etc. The datasources themselves are obviously stateful, but the Data layer classes that interact with them should be stateless and fully independant of one another. In other words, don't cache anything in this layer. It simply serves to abstract away the complexity of the APIs of those data sources (handling API authentication, mapping SQL records to types Kotlin classes, etc)
- Domain layer: where all business logic takes places. It submits/loads data to/from the Data layer, does any necessary processing of that data, and returns results to the UI. If an API should be cached, for example, a Use Case should be responsible for checking the cache and fetching new data if necessary. Furthermore, if one API call depends on another, it may be useful to have Use Cases delegate to other Use Cases. For example, a Use Case to check if the user is logged in, then either return the User Principle object or else navigate to the login screen. Then the Use Case that loads the Dashboard data for a user calls to that other Use Case to get the user object.
- UI layer: comprised of stateless Compose UI (though traditional XML views can work just as well, see this example). Simple screens may not need Ballast VMs, but any screen that's loading data and also maintaining its own state from user interactions probably should have a Ballast VM. These screens should be very basic, delegating nearly everything to a Use Case, and they should not directly access any Data Layer classes.
As far as the Ballast Repository module is concerned, I think I will likely end up removing this module entirely and moving the Cached class and its helper functions to a separate library. I simply could never get the BallastRepository and related functionality to work like I wanted, and found that it just wasn't necessary if you instead use the "Clean Architecture" described briefly above and in more detail in the Android docs.
But I do think the Cached class when used by a Ballast VM is an effective monad, which really helps clean things up when a single screen needs to load data from multiple sources.