All queue management and download state moves from the static DownloadService
companion into an injectable DownloadManager singleton (Koin): the queue,
active downloads, failed list, LiveData, state callbacks and retry loop now
live there. DownloadService becomes a thin lifecycle shell that attaches to
the manager on create and detaches on destroy, keeping only the foreground
notification, Wi-Fi lock and storage monitor. Service start/stop is isolated
in a DownloadServiceController so the manager has no Android context handling.
Also included, needed to make the extracted logic correct and testable:
- removeDownloadedTracksFromList and setSaveFlagForTracks now gate on
Storage.isPathExists instead of getFromPath; with the default java.io
storage root getFromPath returns a wrapper for nonexistent paths, which
wrongly filtered out every track (silently disabling preload downloads).
- downloadStateChangedCallback requeues a RETRYING item before triggering
processNextTracks, so a retry is immediately eligible instead of waiting
for the next callback.
- processNextTracks guards against a detached service (null scope/host),
fixing a potential crash when a task callback fires after destruction.
All former DownloadService.xxx call sites (PlaybackService, NavigationActivity,
SettingsFragment, TrackViewHolder/TrackViewBinder, DownloadUtil, TrackCollectionModel,
DownloadListModel and the Koin modules) now use the injected manager.
Adds DownloadManagerTest (Robolectric) covering enqueue/dedup, downloaded-track
filtering, state reporting, clearDownloads, empty-queue draining, retry
requeuing, failure tracking + delete, and post-detach callback safety.
- deleteAsync removed a failedList entry from the downloadQueue (a no-op:
failed items were already polled out of the queue), leaving stale FAILED
state for deleted tracks. Remove the entry from failedList instead.
- The DownloadTask CoroutineExceptionHandler deleted the complete and
pinned files on any uncaught exception. Those files may already hold a
finalized download (exceptions can occur in post-rename steps such as
the catalog write), so this destroyed finished data. Only the partial
file is deleted now: retries do not append to a possibly untrustworthy
stream, while finalized downloads and cancelled (resumable) downloads
are left untouched.
The metadata size may differ from the bytes actually transferred
(transcoded streams) or from the resumed total, so the raw progress
calculation could leave the 0..100 range. Coerce the reported value
into the valid range.
These discarded null expressions did nothing; the filtering they were
originally meant to perform is handled by the rename blocks and by
DownloadTask.checkIfExists. Removes the misleading dead code.
The music service module was unloaded and reloaded by MusicServiceFactory on
every library access state change, which forced MusicServiceFactory and
ImageLoaderProvider to inject the Koin instance itself and the fetcher
factories to resolve the SubsonicAPIClient lazily from a volatile module.
- MusicServiceFactory now owns the swappable delegates directly: it builds
OfflineMusicService or CachedMusicService(RESTMusicService(client)) for the
current access state, exposes apiClient() alongside getMusicService(), and
keeps resetMusicService() as the explicit hook for server property edits
- the delegate builders are constructor-injectable, keeping the switch logic
testable without the REST stack
- ImageLoaderProvider takes a () -> SubsonicAPIClient provider and rebuilds
the loader when the client instance changes, replacing the ServerID Koin key
- CoverArtFetcher/AvatarFetcher factories resolve the client through the
factory instead of the module
- musicServiceModule is deleted; the DI graph is static and no module is
ever unloaded at runtime
Switching the library between online and offline rebuilt the player
unconditionally (release -> new ExoPlayer -> setMediaItems -> prepare),
which paused audio and even restarted playback after server property edits.
- entering automatic offline is now a pure state flip: the queue and the
player are untouched, unplayable entries are skipped by the playback
failure recovery, and the original order resumes once the server is
reachable again
- returning online does nothing: stream URLs are re-resolved on the next
load through the regenerated music service
- engaging manual offline mode prunes the timeline in place instead of
rebuilding the player: removals never touch the playing item, rebuilt
URIs are applied via replaceMediaItem at adjusted indexes
- the transition decision is extracted into AccessStateTransitions and the
prune plan into OfflineQueuePruner, both unit-tested
- the bus subscription is replaced by a single accessState collector, which
also removes the cross-coroutine serverSelectionChanged flag race and the
pointless player restart on server property edits
- the access state reaction replaces the racy flag with a previous-state
comparison; full player rebuilds remain for JUKEBOX switches, server
switches and the unplayable-current-item fallback
A transient streaming error used to flip the whole library into automatic
offline via reportNetworkFailure(playback=true), which pruned every
non-downloaded item from the queue and stopped playback outright.
- absorb transient connectivity hiccups in the loader with a custom
LoadErrorHandlingPolicy (~45s backoff budget) instead of Media3's default
- on an item failure, recover in place: bounded same-item retries, then skip
to the next item in play order; the queue is never edited
- automatic offline now requires a failed background ping of the selected
server; the playback error alone is no longer evidence of an outage
- drop the now-unreachable PLAYBACK_NETWORK_ERROR classification path and
the playback parameter of reportNetworkFailure
Move ~20 classes from KoinComponent/'by inject()' lookup to constructor
injection, registered in the Koin modules; the dependency graph is now
visible in constructors and these classes are testable without Koin.
- util: CacheCleaner, CommunicationError, DownloadUtil, ContextMenuUtil
are now plain classes provided as singles.
- adapters: Utils object becomes PopupMenuFactory; TrackViewBinder,
HeaderViewBinder, ArtistRowBinder, TextRowBinder, ShareRowBinder,
AlbumRowDelegate/AlbumGridDelegate receive their dependencies (incl.
PopupMenuFactory) via constructor; FolderSelectorBinder drops a dead
KoinComponent.
- service: MusicServiceFactory and ImageLoaderProvider receive the Koin
instance explicitly (they must resolve from the dynamically reloaded
music service module); PlaybackStateSerializer, DownloadTask,
MediaLibrarySessionCallback, CachedMusicService and OfflineMusicService
take their dependencies via constructor.
- subsonic: VideoPlayer becomes an injected class; AvatarFetcher and
CoverArtFetcher take the SubsonicAPIClient, their Coil factories accept
a lazy clientProvider so the client stays current across server
switches.
- models: EditServerModel, GenericListModel and all subclasses get
constructor injection and are wired in the viewModel registrations.
- imageloader: ArtworkBitmapLoader takes the ImageLoaderProvider.
- view: ChatAdapter takes activeServerProvider/imageLoaderProvider.
Service-locator access is confined to Android framework entry points
(UApp, DownloadService, PlaybackService, UltrasonicIntentReceiver,
AlbumArtContentProvider) and the shared Fragment error-handling
extensions; redundant KoinComponent/KoinScopeComponent declarations were
removed from fragments.
Tests: CommunicationErrorReportingTest, AvatarFetcherTest,
CoverArtFetcherTest and OfflineMusicServiceCatalogTest inject mocks
directly instead of booting Koin.
The dedupe marker (lastUpdate) was recorded even when the submission failed,
so re-submitting the identical rating afterwards was silently suppressed.
lastUpdate is now cleared again when the submission fails (only if no newer
rating was recorded meanwhile), while identical updates arriving while a
submission is in flight are still deduplicated. Also makes lastUpdate
@Volatile: it is written from both bus collectors and rating submissions
on different dispatchers.
The early 'activeDownloads.contains -> QUEUED' return made the subsequent
DOWNLOADING/RETRYING branch unreachable: every in-flight download was shown
as queued. QUEUED is now reserved for tracks waiting in the queue.
actualBackend becomes an instance field; it was only read inside the service.
The PlaybackService.instance singleton check in UltrasonicIntentReceiver is
replaced by the MediaControllerProvider connection state, which tracks the
service lifecycle (Connected/Connecting while alive, Disconnected/Failed
otherwise) without global mutable state.
Introduce CoroutinePatterns.loggingScope() and replace all bare
CoroutineScope(Dispatchers.X) delegations and ad-hoc scope creations
with it. Previously a single failed background task in ActiveServerProvider,
PlaybackService, Scrobbler, CacheCleaner, EqualizerController, FileLoggerTree,
ImageLoader, PlayerFragment or TrackViewHolder propagated to the default
uncaught exception handler and crashed the process; now failures are
supervised and logged.
Also removes the redundant ad-hoc IO scope in ArtistRowBinder: executeOn()
already dispatches off the main thread with its own exception handler.
- initiated is now owned by startKoin() alone; setting it before super.onCreate()
made the NavigationActivity Koin-restart fallback unreachable
- isFirstRun is computed on the main thread: NavigationActivity.onCreate can run
before the background initialization completed and read a stale 'false',
silently skipping the welcome dialog on first install
- applicationContext() fails with a descriptive error instead of a bare NPE
when accessed before Application.onCreate
GenericListModel no longer takes a SwipeRefreshLayout: refresh() and
backgroundLoadFromServer(refresh) are View-free, and the loading
indicator is now published via an isRefreshing LiveData that the
observing fragment forwards to its swipe-to-refresh view. Error
handling uses the application context instead of the swipe view's
context. The five list models (Playlist, Podcast, Share, Artist,
Genre) drop the swipe parameter from getItems(); fragments no longer
pass swipeRefresh into model calls and MultiListFragment observes the
model's loading state to drive the indicator.
getServerList() blocked the calling thread (main) while checking and fixing
server indexes on every resume; it now returns a liveData whose first emission
happens after reindexing. addDemoServer() is now a suspend function and the
welcome dialog launches it in lifecycleScope instead of blocking.
birdbird
birdbird
Sam Mingo
Renovate Bot