Singleton sync and chip loader

[mattcarter11]

Feature / Changes

• Include user uploaded songs, albums
• Make syncing of songs, albums, playlists, artist faster and only allow one sync to run (singleton like)
  • Before that the app crashed when loading +7K songs (Songs tab crashes #53)
• Show loading spinner in:
  • Chip when syncing the showed data
  • Liked playlist
• Add a Downloads chip in all old view. This shows albums, artists, playlist that have one or more downloaded song
  • songs now have a dateDownload
  • Fetching of download entities with downloads, ... is purely SQL now = better performance
• Show N download/M songs in playlists/albums/artists
• migrate SongByArtistAsc logic to SQL to improve performance
• Break up DatabaseDao into smaller interfaces to improve development
• Concentrated some SQL selects into one function to centralize code and remove duplicate logic

Sync spinner in chips and liked playlist

New Download chip (show only playlists/arts with any downloaded song)

N/M songs

Fixes

• Fix sort ByPlayCountAsc using count instead of sum

[mattcarter11]

@mikooomich this one has some big files 😅 if anything has to change, I'm more than happy to oblige

[mikooomich]

As in a "this is ready for merge" or suggestions in general? I haven't looked into it too deeply, but looks great so far! A few things I noticed

• App crashes on database migration. You probably can just let auto migration handle it (you basically don't have to do anything for simple field additions), and onPostUpgrade() if necessary. The manual migrations are only really necessary for when auto migrations won't cut it
• On old layout (maybe new too?), switching tabs causes sync(?) even when auto sync is disabled

[mattcarter11]

@mikooomich db migration removed and fixed the ytmSync being ignored in the playlist tab

[mattcarter11]

@mikooomich also, what do you think about adding this? A n/m download count on playlists/artists/albums with any downloaded song.

Maybe add an option to toggle this?

[mikooomich]

You cannot modify old db schema like that. You still need to increment db version and specify a database auto migration, see here -> c3e2ea7#diff-841cb2099b74da4d913f1cf1d60c554d9e6ae0dc6f8c3a22ad3b95a4b5b39568R107

A n/m download count on playlists/artists/albums with any downloaded song. Maybe add an option to toggle this?

Yeah, nice touch

If the pull request is wip could you mark it as such (with the selector), then mark it ready when its ready to be reviewed/merged

[mattcarter11]

@mikooomich change done and added N/M songs

[mikooomich]

Hello. A few nitpicks, otherwise, looks great.

For commits, mostly for sakes of readability/traceability:

• Please remove merge commit(s). PRs should be rebased on conflict, or left as-is
• Remove any rebase fixup commits (those should be applied within commits themselves)
• Squash any duplicates ex. "fix sort lined song"
• All database schema should be contained within one commit. It's just the one change with SongEntity, right? All the json modifications, adding migration, etc can probably go into one commit
• Tagging. You can leave this PR as is, however in the future, we'd very much prefer commits to be tagged according to functionality/category. ex "ui: ..." "syncutils: ...", etc. It just makes searching/skimming/grouping much easier

Download icon is misaligned

NPE crash when clicking on albums

 FATAL EXCEPTION: main
                                                                                                    Process: com.dd3boh.outertunepr63, PID: 4785
                                                                                                    java.lang.NullPointerException: Parameter specified as non-null is null: method androidx.room.RoomSQLiteQuery.bindString, parameter value
                                                                                                    	at androidx.room.RoomSQLiteQuery.bindString(Unknown Source:2)
                                                                                                    	at com.dd3boh.outertune.db.DatabaseDao_Impl.__fetchRelationshipartistAscomDd3bohOutertuneDbEntitiesArtistEntity_1(DatabaseDao_Impl.java:12905)
                                                                                                    	at com.dd3boh.outertune.db.DatabaseDao_Impl.-$$Nest$m__fetchRelationshipartistAscomDd3bohOutertuneDbEntitiesArtistEntity_1(Unknown Source:0)
                                                                                                    	at com.dd3boh.outertune.db.DatabaseDao_Impl$97.call(DatabaseDao_Impl.java:7750)
                                                                                                    	at com.dd3boh.outertune.db.DatabaseDao_Impl$97.call(DatabaseDao_Impl.java:7717)
                                                                                                    	at androidx.room.CoroutinesRoom$Companion$createFlow$1$1$1.invokeSuspend(CoroutinesRoom.kt:129)
                                                                                                    	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
                                                                                                    	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
                                                                                                    	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
                                                                                                    	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:644)
                                                                                                    	at java.lang.Thread.run(Thread.java:1012)
                                                                                                    	Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@b352779, Dispatchers.Main.immediate]
2024-10-20 13:01:55.637  4785-4785  Process                 com.dd3boh.outertunepr63             I  Sending signal. PID: 4785 SIG: 9

[mattcarter11]

@mikooomich

• resolved all comments

• merge removed

• Squash some commits

• alignment fixed

• NPE crash when clicking on albums -> fixed (missing group by album.id)

• All database schema should be contained within one commit -> should i squash commits? what if they are mixed with other stuff?

• Remove any rebase fixup commits (those should be applied within commits themselves) -> what do you mean applied within commits themselves? the commits from the branch used to rebas (dev)?

[mikooomich]

All database schema should be contained within one commit -> should i squash commits? what if they are mixed with other stuff?

"migrate get download logic to SQL (+performance) and add isDownload to song table"
For this, i'd seperate the rest of the sql stuff (another commit) and add is downloaded (one commit) . When you add the isdownloaded field, it comes it side effects: you need to handle migration and room will generate the json for db V16. that should all be contained in that commit.

This also means squash the following since into that commit since that will be handled in the commit you add isdownloaded

• remove db migration
• add new versiondb file
• re-add migration since auto was failing
• revert 15.json changes
• and any more i'm missing (?)

Remove any rebase fixup commits (those should be applied within commits themselves) -> what do you mean applied within commits themselves? the commits from the branch used to rebas (dev)?

"add missing import after rebase" should not be its own commit (unless it's alot of fixing and makes sense to otherwise), instead, add it into the commit that needs it. (if you don't know where it came from, honestly you can just leave it as it)

[mattcarter11]

@mikooomich changes done