Clean up separation between app/database/network layers.

[johnjohndoe]

Description

• Clean up separation between app/database/network layers for Session and Meta model classes.
  • network models are only used in the context of fetching data from the network and parsing it.
  • database models are only used in the context of loading data from the database.
  • Both network and database models are only used in AppRepository which acts as central junction for converting models between the layers.
  • For the app layer AppRepository only exposes data as app models.
• Remove unneeded :app:Highlight model class.
• Increase test coverage for Session#sanitize extension function.
• Trim white space when parsing speakers string.

Successfully tested on

with ccc37c3 flavor, debug build

• ✔️ Samsung Galaxy Tab S7 FE, SM-T733 (tablet), Android 14 (API 34)

[github-actions]

Test results (test)

 85 files  + 2   85 suites  +2   10s ⏱️ -1s
793 tests +27  793 ✅ +27  0 💤 ±0  0 ❌ ±0 
820 runs  +27  820 ✅ +27  0 💤 ±0  0 ❌ ±0 

Results for commit 96214ab. ± Comparison against base commit a1bbad1.

This pull request removes 6 and adds 33 tests. Note that renamed tests count towards both.

nerd.tuxmobil.fahrplan.congress.dataconverters.HighlightExtensionsTest ‑ toHighlightAppModel()
nerd.tuxmobil.fahrplan.congress.dataconverters.SessionExtensionsTest ‑ toSessionAppModel returns an app session derived from a network session()
nerd.tuxmobil.fahrplan.congress.dataconverters.SessionExtensionsTest ‑ toSessionDatabaseModel returns a database session derived from an app session()
nerd.tuxmobil.fahrplan.congress.dataconverters.ShiftExtensionsTest ‑ toSessionAppModel sessionizes a shift and duration returns correct value in minutes()
nerd.tuxmobil.fahrplan.congress.dataconverters.ShiftExtensionsTest ‑ toSessionAppModel sessionizes a shift and handles time zone correctly()
nerd.tuxmobil.fahrplan.congress.models.SessionTest ‑ cancel marks a session as canceled and resets all change other flags()

info.metadude.android.eventfahrplan.database.models.SessionTest ‑ 10: changedTitle = false, changedSubtitle = false, changedRoomName = false, changedDayIndex = false, changedStartTime = false, changedDuration = false, changedSpeakers = false, changedLanguage = false, changedRecordingOptOut = true, changedTrack = false -> isChanged = true
info.metadude.android.eventfahrplan.database.models.SessionTest ‑ 1: changedTitle = false, changedSubtitle = false, changedRoomName = false, changedDayIndex = false, changedStartTime = false, changedDuration = false, changedSpeakers = false, changedLanguage = false, changedRecordingOptOut = false, changedTrack = false -> isChanged = false
info.metadude.android.eventfahrplan.database.models.SessionTest ‑ 2: changedTitle = true, changedSubtitle = false, changedRoomName = false, changedDayIndex = false, changedStartTime = false, changedDuration = false, changedSpeakers = false, changedLanguage = false, changedRecordingOptOut = false, changedTrack = false -> isChanged = true
info.metadude.android.eventfahrplan.database.models.SessionTest ‑ 3: changedTitle = false, changedSubtitle = true, changedRoomName = false, changedDayIndex = false, changedStartTime = false, changedDuration = false, changedSpeakers = false, changedLanguage = false, changedRecordingOptOut = false, changedTrack = false -> isChanged = true
info.metadude.android.eventfahrplan.database.models.SessionTest ‑ 4: changedTitle = false, changedSubtitle = false, changedRoomName = true, changedDayIndex = false, changedStartTime = false, changedDuration = false, changedSpeakers = false, changedLanguage = false, changedRecordingOptOut = false, changedTrack = false -> isChanged = true
info.metadude.android.eventfahrplan.database.models.SessionTest ‑ 5: changedTitle = false, changedSubtitle = false, changedRoomName = false, changedDayIndex = true, changedStartTime = false, changedDuration = false, changedSpeakers = false, changedLanguage = false, changedRecordingOptOut = false, changedTrack = false -> isChanged = true
info.metadude.android.eventfahrplan.database.models.SessionTest ‑ 6: changedTitle = false, changedSubtitle = false, changedRoomName = false, changedDayIndex = false, changedStartTime = true, changedDuration = false, changedSpeakers = false, changedLanguage = false, changedRecordingOptOut = false, changedTrack = false -> isChanged = true
info.metadude.android.eventfahrplan.database.models.SessionTest ‑ 7: changedTitle = false, changedSubtitle = false, changedRoomName = false, changedDayIndex = false, changedStartTime = false, changedDuration = true, changedSpeakers = false, changedLanguage = false, changedRecordingOptOut = false, changedTrack = false -> isChanged = true
info.metadude.android.eventfahrplan.database.models.SessionTest ‑ 8: changedTitle = false, changedSubtitle = false, changedRoomName = false, changedDayIndex = false, changedStartTime = false, changedDuration = false, changedSpeakers = true, changedLanguage = false, changedRecordingOptOut = false, changedTrack = false -> isChanged = true
info.metadude.android.eventfahrplan.database.models.SessionTest ‑ 9: changedTitle = false, changedSubtitle = false, changedRoomName = false, changedDayIndex = false, changedStartTime = false, changedDuration = false, changedSpeakers = false, changedLanguage = true, changedRecordingOptOut = false, changedTrack = false -> isChanged = true
…

♻️ This comment has been updated with latest results.

[github-actions]

Test results (androidTest), API level 26

4 tests  ±0   4 ✅ ±0   0s ⏱️ ±0s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 96214ab. ± Comparison against base commit a1bbad1.

♻️ This comment has been updated with latest results.

[johnjohndoe]

@cketti If you have the energy to look into this as well - then you are welcome. No pressure!

[cketti]

I think using the names SessionAppModel, SessionDatabaseModel, and SessionNetworkModel is a good (first) step. However, I wish you renamed the classes instead of only using import aliases.

Having different names clearly shows there's a lot of conversions going on between the different model classes. I think this could be avoided and the code would be simplified by having a single Session class in an API module. Then instead of calling it the "network" module, you think of it as a data provider module where the network access and source data format are just implementation details. The data provider module would parse and convert the source data to Session instances internally and only pass those to the app.
Similarly, the database module could use that Session class for communication with the app and do all necessary conversions internally.
UI code probably shouldn't use this class, but have specialized UI models that contain exactly the data that is necessary for a particular screen.

[johnjohndoe]

Thank you for your feedback!

In my understanding is that the AppRepository is the data provider for the UI / app layer. It references the network and database layers to fulfill its purpose and to hide implementation details like the database and network models from the app layer. The goal of this pull request is to prevent the AppRepository from exposing anything other than the app layer model (to the best of its ability).

In its function of loading and converting data I try to keep the AppRepository as "uninvolved" as possible. Any screen-specific data conversion is done in the view model of that screen. - There is a temptation to move this kind of data preparation from a view model into the AppRepository. I think this would tell the AppRepository too much about the screen and is not a good separation of concerns.

I am happy to brainstorm ideas to improve the architecture. This probably works better in a synchronous conversation. Let's continue apart from this merge request.

[johnjohndoe]

@cketti Are you fine with resolving the remaining review comments?