[Discussion] Absolute Unit Refactor

[xxxserxxx]

The branch xxxserxxx contains a massive refactoring aimed to address a number of fundamental design issues in stmp (inherited by stmps). I intend this issue to be a discussion thread about it.

1. stmp blithely interchanged Subsonic's filesystem-based Directory model with the metadata-based Author/Artist model. With some servers (Navidrome, for example) this was usually not noticable because IDs were fungible between the two models. With other servers (such as gonic) this often caused confusing behaviors when browsing. In all cases, it was possible to get stmp(s) into an odd situation, such as navigating into an album in the Entity list and then repeatedly navigating back up the tree via [..] until the Entity column looked like the Artists column.
2. The browser columns were blatant lies. The Artist column only contained artists if the music directory filesystem strictly followed a layout where artists were at the top, and every artist contained albums, and albums contained only songs. If there was any variation, you could see albums in the Artists column or artists in the Albums column. And the code could never know what, exactly, it was looking at.
3. The code itself followed some bad conventions; in particular, all of the stucts in the subsonic library were named with the convention SubsonicXXXX; this is considered poor Go form, where type and method names are not supposed to repeat the code structure.
4. The subsonic library had a bad habit of not being type safe: every method returned a SubsonicResponse pointer, which could be any one of several subtypes depending on the data from which it was unmarshalled. In Go, this sort of situation is hard to solve and requires either much code and data duplication or type-unsafeness. It is not solvable with generics (most problems generics are designed to address are not addressable with Go generics). Regardless, the library exacerbated this by having all methods, regardless of subtype, return SubsonicResponse structs, leaving type checking to the caller. This was error-prone.
5. Because of the first and second issues, as code based on metadata was changed to add features, bugs related to the lack of consistency between use of Directory and Artist/Album APIs were starting to appear. Navidrome would hide these issues as (e.g.) Artist IDs were also valid Directory IDs, but under other servers (e.g. gonic), this would cause items to suddenly disappear in the UI, and other odd behavior.

Thus, the xxxserxxx branch. The very first commit addressed issues 1-3:

• The subsonic library was refactored to remove all Subsonic prefixes from types; the unnecessarily repetitive subsonic.SubsonicArtist became subsonic.Artist.
• Almost all methods now return more strict types. GetArtist() returns an Artist; GetAlbum() returns an Album.
• BrowserPage now operates entirely on the metadata methods, eschewing where possible the Directory and Index functions. This enforces consistency across the application. Almost all of the rest of the pages and code already used the metadata functions, so the biggest impact was on BrowserPage.

Following commits are of four sorts:

• Because there were many PRs stacking up and a mess of difficult merges, all of my PRs have been merged onto this branch:
  • fix: issue #88 #91
  • adds: ability to toggle song info panel on queue page via i #90 (code provided by @mahmed2000; I just created the branch and submitted the PR)
  • Synced lyrics support #87
  • Feat 52 search by genre #80
• Bug fixes related to the merge; tweaks to make stmps behave consistently whether the server is Navidrome or gonic; and performance enhancements such as the removal of redundant Sort calls.
• Bug fixes from the issue list (this is a sort of subset of the first item, because there are at least two unmerged PRs that only deal with fixing user-reported bugs)
• Merges of other people's PRs, and future enhancements

At the point of commit 3359cc5, changes fall into the first two categories. My next activities on this branch are the following -- the first two items are my first priority; the rest are in no particular order:

• Add a mode where BrowserPage can be used as a directory browser, using Indexes and Directory API calls. These calls are reflections of the filesystem, and operate without looking at metadata; they represent artist/album hierarchy only so far as the users have been consistent in their directory structure. Users who don't put albums under artists will see a completely different view of their data than the metadata view. This view, while silly, is necessary to catch music which might be mis-tagged or missing tags.
• Merges of other people's PRs.
• Continued fixes of reported issues
• Merge the concurrent cover art enhancement
• Add LRU control over albums and cover art, to prevent stmps memory caching from slowly eating all system memory. The code for this exists; I just haven't hooked it into anything yet.
• Performance enhancements; in particular some areas could benefit from binary search instead of the current linear search (e.g. synchronized lyrics)
• Re-add the log-to-disk function that was part of, and then redacted out of, an unrelated PR.
• Support for log levels, to control verbosity
• Add a mocking library and start filling in unit tests
• Factor out the proactive caching of playlist data. Playlists should behave like all other tree data: load the children when the user navigates to it. There's discussion about this decision in Albums start showing up as blank after clicking through other ones #85
• Expand the search/filtering function. / should work in the Entities column, and also in the playlists, search, and genre search pages
• Update screenshots
• Improve the command line arguments; they're poorly documented and kinda ugly. I'd kind of like to replace viper with claptrap, but I'm biased. Changing that would be low priority, but improving the CLI help text is valuable.

I want a way to make managing these merges easier. @spezifisch is often busy for periods of time, and I have spurts of productivity, and it's easy for me to pile up several non-trivial PRs that become overwhelming and hard to merge. I've been using jujutsu on my desktop, and it really does massively improve over git's notoriously terrible merging, and is nearly as easy as Mercurial to work with... but that doesn't address the fundamental procedural issue. My solution, at the moment, is to keep my own "main" -- that's this xxxserxxx branch. I still do not want to hard-fork stmps, so I'm open to suggestions.

BTW, I'd like to try to recruit @mahmed2000, who obviously has a better handle on tview than myself and has already been immensely helpful; having another contributor would be fantastic, and from what I've seen, they'd bring a lot of value.

There's a lot of discussion that includes history and observations in #85.