MergedCollectionMap refactor

[Arlodotexe]

Overview

The MergedCollectionMap is a class which contains all of our logic for merging collections together. It's a very important part of the AdapterModel layer, where we merge many CoreModel instances into a single AppModel instance.

The existing code for this was created in Nov 2020, and while it mostly works, after using it for a while we've found some improvements that need to be made.

General notes

• A "collection interface" refers to an interface such as ITrackCollection, IAlbumCollection, IImageCollection, etc.
• Some AppModels such as ILibrary, IArtist or ITrack implement more than one collection interface.
• Each instance of MergedCollectionMap handles a single collection interface at a time.
• MergedCollectionMap is fully generic, allowing us to reuse the code for all collection interface implementations in AdapterModels.

Brainstorm session 4/23/2022

The follow are notes from a brainstorm session between @Arlodotexe and @amaid on how to best handle the issues that arise from merging multiple collections into one collection.

Possible approaches

Sparse collection population

• Lazy load, but smarter
  • Use IAsyncEnumerable instead of Task<List> (implemented)
  • Use an event to notify sources changed (implemented)
• For a mergeable item, Sources property is incomplete until we've seen all items.
• Asking for items in the middle of the collection would require you to get all preceing items first.
• ItemsCount is invalid until we've seen everything.

Full collection population (preload everything)

• Preload items
  • Load/merge all contents of a collection before returning
  • Data affected by merging in new items would always be accurate
  • Significantly slower. For every collection in a collection, needs to load all/merge items before returning to get accurate merged data.
    • Example: When getting a Playlist from a PlaylistCollection, in order for Playlist.TotalTrackCount to be accurate, need to get/merge all possible tracks.
  • Renders IAsyncEnumerable pointless, everything is preloaded already
  • No need for sources changed event, sources are accurate before being returned, and would never change Required for adding/removing cores while app is running.

Hybrid (background preload)

• Begin populating from all sources when the MergedCollectionMap is created
• Optionally emit data as soon as it becomes available (user's choice)
• Always emit the Count changed event
• If data is changed on a core while scan is running, pause the scan and accommodate it (IAsyncEnumerable makes this really easy)
• Once scan is complete, all items are merged and the item count is accurate
• Would play REALLY nicely with a caching plugin, as the cache would be updated as the MergedCollectionMap is populated.
• Minimizes the odds that the user will encounter problems caused by incorrect item counts, but doesn't eliminate it completely.
• Maybe also add InitAsync to all AppModels, if you want to wait for them to be fully merged.

Questions

• For data that is potentially wrong until we have all items merged into it, is it acceptable behavior for the data to be wrong on returned items until lazy init has finished and merged everything?
  • Yes for UI, but it's easier to write broken code (not checking the items count during iteration, getting items in an ItemsCount handler). Must be mindful of these when deciding data behavior.
  • For headless/backend only - Maybe? As long as the dev knows that the count can change when getting items.
• What to choose?
  • The hybrid approach seems to be the best solution.

What to do

• [ ] Clean up code codebase
• [ ] Get sparse collection loading working as expected
• [ ] Add in the background preloading
• [ ] Refactor AddSource and RemoveSource on IMergedMutable to be asynchronous
• [ ] Re-evaluate what should happen to existing data when a new source is added.
• [ ] Implement search results and other missing types in MergedCollectionMap
• [ ] Write unit tests.
• [ ] Write lots of unit tests.

[Arlodotexe]

Since sparse collection population changes the count as it merges items, and getting items causes sparse collection population, it's easy to create a stack overflow by getting items inside the count changed event, like we ran into here for the Zune Desktop shell.