The Big Refactoring
Current situation
Whisperfish 0.6 inherited a lot of things from 0.5. We tried to recreate the surface of the Go-based application, and this gave us a fragile spaghetti of Rust actors and methods. For instance, we have
"Actors"
a whole lot of actors, but they are not really real actors:
- MessageActor, SessionActor, which are basically forwarding requests for their MessageModel, SessionModel QML-counterparts.
- ClientActor, which is more of a real actor: it tracks the login state. It's becoming a God actor as we move on though, with
client.rs
going over 1kLOC. - Reloading messages or sessions involves sending a message to an actor, and the actor storing it in a QBoxObject. This results in quite some callback hell.
- They all store their state in
Option
s.
"Models"
a whole lot of Models, which aren't really models. They are a proxy object implementing QAbstractListModel
.
Moreover, all "Models" act like singletons, which means that Rust and QML needs to be in sync about which Session is currently visible.
Problem statement
We have acquired quite a bit of spaghetti, and the technical debt that comes with it. Keeping the sync between QML and Rust has become a chore. For instance, implementing something simple as linked devices requires quite some thought as to where to put the relevant handlers, signals, and properties.
Apart from spaghetti, there's also a lot of boilerplate in the QML-Rust-QML chain, especially passing through the many actors.
There's a lot of Box::new(async move {}.into_actor(self).map())
stuff going on, which we should hide behind some abstraction.
Singleton models make it difficult to keep Rust and QML in sync.
Proposals
- Application state tracking and synchronisation. A proxy object (thanks for the suggestion, @ntninja) should track whether the application is
Locked
orUnlocked
, and state transitions should be tracked in QML and Rust synchronously. FromLocked
, we can enter the registration phase (or the two different registration states, after !77 (closed)), etc. - No global Models any more. QML needs to fetch the models from the general application state, in Unlocked state.
- Models are PODs, they do not have a
reload
method. Reloading should, in fact, not be necessary, except for maybeDeviceModel
. Even then, this should pass by the application state, which delegates it to Client. - Get rid of the Singletons, no global SessionModel and DeviceModel. Instead, we register the Models as types for QML, and have the client/application state return a list of messages from the session. This has as benefit that tracking the active session is done through the fact that there's an instance of the session.
- Only start Actors when their state is at least initialised (
Option<Storage>
->Storage
, ...)
Discussion
I invite @mjtorn and @ntninja (and any one else interested) to this discussion. Best case, we can generalise some a lot of things and move them to https://gitlab.com/rubdos/sailo-rs and https://github.com/woboq/qmetaobject-rs/. The former would be for general Sailfish-Rust(-Tokio/Actix) things, the latter does accept pull requests for generic async Qt/QML support.