Investigate RemoteMediator for Notifications

[uwemartin-lgtm]

What does this do?

Apply the Paging3 library including a remote mediator and updating the Notification view model to use a flow-based approach.

Why is this needed?

See ticket T403106

Technical background

Current implementation

The current implementation uses an in-memory sorting and filtering. The Room database is used for storing all notifications received from the server, but the app reloads ALL of that data into memory before commencing the filtering and sorting.
The current implementation does reuse pages already stored in the database but it does not persist the pagination token. The database is only reset when the user logs out.

Paging3 / RemoteMediator

The library intends to only load a fraction of the data necessary to display.
I noticed that (in the intended scenario) it loads a couple of pages (in this case 4 pages with 50 elements).
When the user scrolls to the end of the available data, the library coordinates loading of more data and buffering in the local database. Following the functionality of the current implementation where ALL data is loaded to the local database, I updated the NotificationRemoteMediator implementation to also implement the loading and local storing of all notifications from the server (call to syncAll). This means that the refactored code does not take full advantage of the Paging3 library (but easily could if deviation of from functionality from current implementation is preferred).

Overview of architectural update

Architecture of current implementation

#### Architecture of refactored implementation ####

Main architectural difference

The filtering/sorting logic moves from the in-memory-processing in the view model as done in the legacy code to the database DAO which provides the PagingSource.
This made the database query significantly more complex. but still feasible in terms of performance.
Implementing a custom paging source logic would also require implementing a custom invalidation pipeline.
Based on feedback to this PR or with a separate PR, I can also look into that.
The refactored implementation also does not check the availability of the server using WikipediaApp.instance.isOnline
It relies on the remote mediator exception handling.
The refactored code introduces persistence for the pagination token (although the token has to be reset to null during initial load to avoid ignoring "fresh" pages - see open points).
The search bar is now added into the NotificationContainerList by the view model instead of relying on the Activity. This results also in slightly different results for unit testing.

Introduction of interfaces

The PR introduces a set of interfaces to enable mocking:

• NotificationViewModel: allowing flexible use of current implementation "legacy" and refactored code for unit testing exclusively.
• NotificationRepository: allowing mock ("fake") repositories for unit testing and instrumented (performance) tests
• NotificationFilterHelper: allowing mocking of reference wikis and categories/types.
• NotificationPreferences: allowing mocking of user selection for filtering

Main updates to git repo

• renamed the existing view model to NotificationLegacyViewModelImpl
• introducing a new view model NotficationRefactoredViewModelImpl
• introducing interface and related implementation for NotificationRepository, NotificationFilterHelper and NotificationPreferences
• added unit tests (see below)
• added instrumented tests and related mock data (see below)
• added entity and DAO for the remote key (paging token) persistence.
• added a factory for the notification view model (NotificationViewModelFactory)

Change of functionality

The refactored application uses the following approach for updating the counts on the "All" and "Mentions" tab: With each page loaded, the count increases (also visible). As long as the end of the data has not been reached, a "+"-sign is added to the counts to indicate the number of notifications currently loaded. When all data is loaded, the numbers are shown without a "+" sign. This approach would also allow for (if desired) only load a subset of all notifications from the server and making this visible to the user.

Testing

The PR contains both unit testing (running on developer machine) and performance testing (instrumented tests to run on physical device).

Unit testing

The unit testing focusses on checking the filter and sorting functionality. Based on the value of legacy it can run the current implementation or the refactored code to facilitate comparison.
In addition to the filter and sorting functionality, the unit test cases check the refactored code for

• dynamic removal of search bar in multi-selection mode
• reactivity of counts for "all" and "mentions"

Performance testing

Purpose: Evaluate the performance of the refactored code compared to the current implementation.
The tests reside in different test classes in the androidTest folder (combining them did not make sense due to the difference in code structure). However, the tests use a common stimuli generator which resides as a separate class in the same folder.
For the performance testing I used 1000 notifications in 100 iterations with filter setting that filtered down to 28+1 notifications (legacy code only provides 28).
The detailed results are mentioned in the linked Phabricator ticket. Basically, the median execution time went from 140 milliseconds to around 120 milliseconds.

Implementation updates

• adding room paging dependency to app/build.gradle and related entry in libs.versions.toml
• extending the database tests in AppDatabaseTest.kt to cover also the additional table for the remote key persistence
• adding three constants to Constants.kt which are used across several files
• Incremented the database version in AppDatabase.kt and added migration scheme. Required for persistence of the remote key.
• Updating MwQueryResponse.kt to access information whether the end of pagination has already been reached with one specific request: Adding the „notification continue“ entry notcontinue.
• Updating the NotificationActivity in various places to handle the flow-based view model. The activity now listens to both uiState and notificationFlow provided by the view model. Changed to using Paging adapter for RecyclerView introducing a custom diffing class. Also, communication with the NotificationFilterActivity has been extended to discover changes of user filter preferences (potential bug in current implementation). Furthermore, update of the loading indicator Progressbar had to be adapted.
• Updates to NotificationFilterActivity: Extending communication with NotificationActivity Usage of of NotificationFilterHelper.
• Made small updates to allow for testing to the current implementation of NotificationViewModel and renamed the file to NotificationViewModelImpl.
• Changed NotificationRepository into an interface where the implementation resides in NotificationRepositoryImpl
• Changed NotificationRepository into an interface where the implementation resides in NotificationRefactoredRepositoryImpl and NotificationLegacyRepositoryImpl
• Added equals and hashCode functionality to Notification.kt because the diffing class in the activity needs that.
• Updated the notification DAO with some really ugly SQL queries.

Provisions for manual testing

In NotificationRepositoryImpl.kt you can override the usage of the API to retrieve data from the server and instead use mock data to experience the dynamic when using the app.

• Enable the code from line 39 to line 73 starting with:
• /* --- MOCK DATA GENERATOR FOR MANUAL TESTING ---
• And enable the code from line 112 to 135 starting with
• /* --- OVERRIDE FOR MANUAL TESTING ---

Open questions/issues/todos

Pending review comments/maturity of this PR:

• Remove legacy code incl. redundancy of NotificationHelper (separate implementation file and companion object in NotificationHelperActivity due to usage in legacy code)
• Additional logic to avoid reloading all pages on startup: Currently all data is loaded to avoid losing "fresh" data. Requires a comparison of tokens with a previously stored "start token" (which needs to be stored in the local database additionally)
• If the project decides against such an approach, Room table for pagination token can be replaced with a simple data store...
• Ugly SQL queries: The queries are functional, but aesthetically challenging... Can the project live with that or do I need to look into custom PagingSource
• End-to-end testing: Do we have some possibility here or do I need to build the infrastructure by myself?

Phabricator:
https://phabricator.wikimedia.org/T...