mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-22 07:15:47 +00:00
refactor(network): decouple network-reachability layer from features → lib/network
The connection-reachability + Subsonic network-guard helpers were pinned low by four lib/api consumers (subsonicLibrary/Playlists/Ratings/Scrobble) yet ran two lower-layer→feature inversions. Both removed without a registry: - devOfflineBrowseStore is a self-contained DEV-only toggle with zero offline- feature coupling — it was only colocated there. Relocated features/offline/store → store/ (global). The @/features/offline barrel re-exports it so feature/UI consumers are unchanged; the three lower-layer readers (subsonicNetworkGuard, activeServerReachability, useConnectionStatus) now import it from @/store directly. - subsonicNetworkGuard's only other feature dep was playback's resolvePlaybackUrl, used solely for the psysonic-local:// skip check. Added hasLocalPlaybackUrl to the existing M4 substrate store/localPlaybackResolve — it mirrors resolvePlaybackUrl's local-source branch exactly (same profile resolution; the empty-serverId playback fallback never applies in the guard), so the skip stays bit-identical. network/ (subsonicNetworkGuard + activeServerReachability + tests) → lib/network, now @/features-free. useConnectionStatus is now iron-rule-clean and stays in hooks/ (cross-cutting). Test mocks retargeted to the new seam modules. Behavior-adjacent (covered by suite; default paths identical): the local-bytes skip helper — flag for offline-playback runtime QA alongside the M4 media-resolver seam.
This commit is contained in:
@@ -0,0 +1,46 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { useDevOfflineBrowseStore } from '@/store/devOfflineBrowseStore';
|
||||
import {
|
||||
getActiveServerReachable,
|
||||
isActiveServerReachable,
|
||||
onActiveServerBecameReachable,
|
||||
resetActiveServerConnectionSnapshot,
|
||||
setActiveServerReachable,
|
||||
} from '@/lib/network/activeServerReachability';
|
||||
|
||||
describe('activeServerReachability', () => {
|
||||
beforeEach(() => {
|
||||
useDevOfflineBrowseStore.setState({ forceOffline: false });
|
||||
resetActiveServerConnectionSnapshot();
|
||||
});
|
||||
|
||||
it('isActiveServerReachable requires an explicit successful probe', () => {
|
||||
expect(isActiveServerReachable()).toBe(false);
|
||||
setActiveServerReachable(true);
|
||||
expect(isActiveServerReachable()).toBe(true);
|
||||
setActiveServerReachable(false);
|
||||
expect(isActiveServerReachable()).toBe(false);
|
||||
});
|
||||
|
||||
it('exposes the last probe result', () => {
|
||||
setActiveServerReachable(true);
|
||||
expect(getActiveServerReachable()).toBe(true);
|
||||
});
|
||||
|
||||
it('isActiveServerReachable is false when DEV force-offline is enabled', () => {
|
||||
if (!import.meta.env.DEV) return;
|
||||
setActiveServerReachable(true);
|
||||
useDevOfflineBrowseStore.setState({ forceOffline: true });
|
||||
expect(isActiveServerReachable()).toBe(false);
|
||||
});
|
||||
|
||||
it('onActiveServerBecameReachable fires only on false/null → true', () => {
|
||||
const listener = vi.fn();
|
||||
onActiveServerBecameReachable(listener);
|
||||
setActiveServerReachable(false);
|
||||
setActiveServerReachable(true);
|
||||
expect(listener).toHaveBeenCalledTimes(1);
|
||||
setActiveServerReachable(true);
|
||||
expect(listener).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,61 @@
|
||||
import { isDevOfflineBrowseForced } from '@/store/devOfflineBrowseStore';
|
||||
|
||||
export type ConnectionStatus = 'connected' | 'disconnected' | 'checking';
|
||||
|
||||
/**
|
||||
* Active-server reachability snapshot maintained by `useConnectionStatus`.
|
||||
* Non-hook code (queue sync, favorites refresh) uses this to avoid noisy
|
||||
* network attempts while the browser or Subsonic endpoint is down.
|
||||
*/
|
||||
let activeServerReachable: boolean | null = null;
|
||||
let connectionStatus: ConnectionStatus = 'checking';
|
||||
|
||||
const reachableListeners = new Set<() => void>();
|
||||
const connectionStatusListeners = new Set<() => void>();
|
||||
|
||||
/** Fires when the active server transitions to reachable (`false`/`null` → `true`). */
|
||||
export function onActiveServerBecameReachable(listener: () => void): () => void {
|
||||
reachableListeners.add(listener);
|
||||
return () => reachableListeners.delete(listener);
|
||||
}
|
||||
|
||||
export function setActiveServerReachable(ok: boolean | null): void {
|
||||
const wasReachable = activeServerReachable === true;
|
||||
activeServerReachable = ok;
|
||||
if (!wasReachable && ok === true) {
|
||||
for (const listener of reachableListeners) listener();
|
||||
}
|
||||
}
|
||||
|
||||
export function getActiveServerReachable(): boolean | null {
|
||||
return activeServerReachable;
|
||||
}
|
||||
|
||||
/** Shared across all `useConnectionStatus` hook instances (manual retry, polling). */
|
||||
export function getConnectionStatus(): ConnectionStatus {
|
||||
return connectionStatus;
|
||||
}
|
||||
|
||||
export function setConnectionStatus(next: ConnectionStatus): void {
|
||||
if (connectionStatus === next) return;
|
||||
connectionStatus = next;
|
||||
for (const listener of connectionStatusListeners) listener();
|
||||
}
|
||||
|
||||
export function subscribeConnectionStatus(listener: () => void): () => void {
|
||||
connectionStatusListeners.add(listener);
|
||||
return () => connectionStatusListeners.delete(listener);
|
||||
}
|
||||
|
||||
/** Test helper — resets module-level connection snapshot. */
|
||||
export function resetActiveServerConnectionSnapshot(): void {
|
||||
activeServerReachable = null;
|
||||
connectionStatus = 'checking';
|
||||
}
|
||||
|
||||
/** True only when the browser is online and the last active-server probe succeeded. */
|
||||
export function isActiveServerReachable(): boolean {
|
||||
if (isDevOfflineBrowseForced()) return false;
|
||||
if (typeof navigator !== 'undefined' && !navigator.onLine) return false;
|
||||
return activeServerReachable === true;
|
||||
}
|
||||
@@ -0,0 +1,92 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { useAuthStore } from '@/store/authStore';
|
||||
import { resetAuthStore } from '@/test/helpers/storeReset';
|
||||
import { setActiveServerReachable } from '@/lib/network/activeServerReachability';
|
||||
import { shouldAttemptSubsonicForServer } from '@/lib/network/subsonicNetworkGuard';
|
||||
|
||||
const hasLocalPlaybackUrlMock = vi.fn((_trackId: string, _serverId: string) => false);
|
||||
|
||||
vi.mock('@/store/localPlaybackResolve', () => ({
|
||||
hasLocalPlaybackUrl: (trackId: string, serverId: string) =>
|
||||
hasLocalPlaybackUrlMock(trackId, serverId),
|
||||
}));
|
||||
|
||||
describe('shouldAttemptSubsonicForServer', () => {
|
||||
beforeEach(() => {
|
||||
resetAuthStore();
|
||||
setActiveServerReachable(null);
|
||||
hasLocalPlaybackUrlMock.mockReturnValue(false);
|
||||
});
|
||||
|
||||
it('returns false without a server id', () => {
|
||||
expect(shouldAttemptSubsonicForServer('')).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when the active server probe failed', () => {
|
||||
const activeId = useAuthStore.getState().addServer({
|
||||
name: 'Active',
|
||||
url: 'http://active.test',
|
||||
username: 'u',
|
||||
password: 'p',
|
||||
});
|
||||
useAuthStore.getState().setActiveServer(activeId);
|
||||
setActiveServerReachable(false);
|
||||
expect(shouldAttemptSubsonicForServer(activeId, 't1')).toBe(false);
|
||||
});
|
||||
|
||||
it('allows a non-active playback server when the active probe failed', () => {
|
||||
const playbackId = useAuthStore.getState().addServer({
|
||||
name: 'Playback',
|
||||
url: 'http://playback.test',
|
||||
username: 'u',
|
||||
password: 'p',
|
||||
});
|
||||
const activeId = useAuthStore.getState().addServer({
|
||||
name: 'Browse',
|
||||
url: 'http://browse.test',
|
||||
username: 'u',
|
||||
password: 'p',
|
||||
});
|
||||
useAuthStore.getState().setActiveServer(activeId);
|
||||
setActiveServerReachable(false);
|
||||
expect(shouldAttemptSubsonicForServer(playbackId, 't1')).toBe(true);
|
||||
expect(shouldAttemptSubsonicForServer(playbackId)).toBe(true);
|
||||
expect(shouldAttemptSubsonicForServer(activeId)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when the track resolves to a local playback url', () => {
|
||||
setActiveServerReachable(true);
|
||||
hasLocalPlaybackUrlMock.mockReturnValue(true);
|
||||
expect(shouldAttemptSubsonicForServer('srv-1', 't1')).toBe(false);
|
||||
expect(hasLocalPlaybackUrlMock).toHaveBeenCalledWith('t1', 'srv-1');
|
||||
});
|
||||
|
||||
it('returns true for stream playback when the active server is reachable', () => {
|
||||
const activeId = useAuthStore.getState().addServer({
|
||||
name: 'Active',
|
||||
url: 'http://active.test',
|
||||
username: 'u',
|
||||
password: 'p',
|
||||
});
|
||||
useAuthStore.getState().setActiveServer(activeId);
|
||||
setActiveServerReachable(true);
|
||||
expect(shouldAttemptSubsonicForServer(activeId, 't1')).toBe(true);
|
||||
});
|
||||
|
||||
it('bypasses the local-url skip when called without a trackId (metadata gate)', () => {
|
||||
const activeId = useAuthStore.getState().addServer({
|
||||
name: 'Active',
|
||||
url: 'http://active.test',
|
||||
username: 'u',
|
||||
password: 'p',
|
||||
});
|
||||
useAuthStore.getState().setActiveServer(activeId);
|
||||
setActiveServerReachable(true);
|
||||
hasLocalPlaybackUrlMock.mockReturnValue(true);
|
||||
// Byte-style call (with the track id) is blocked because the bytes are local…
|
||||
expect(shouldAttemptSubsonicForServer(activeId, 't1')).toBe(false);
|
||||
// …but the metadata gate omits the track id, so it never consults
|
||||
// hasLocalPlaybackUrl and stays allowed while the server is reachable.
|
||||
expect(shouldAttemptSubsonicForServer(activeId)).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,46 @@
|
||||
import { useAuthStore } from '@/store/authStore';
|
||||
import { hasLocalPlaybackUrl } from '@/store/localPlaybackResolve';
|
||||
import { isDevOfflineBrowseForced } from '@/store/devOfflineBrowseStore';
|
||||
import { resolveServerIdForIndexKey } from '@/lib/server/serverLookup';
|
||||
import { isActiveServerReachable } from '@/lib/network/activeServerReachability';
|
||||
|
||||
function isSameServerProfile(a: string, b: string): boolean {
|
||||
if (!a || !b) return false;
|
||||
if (a === b) return true;
|
||||
return resolveServerIdForIndexKey(a) === resolveServerIdForIndexKey(b);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `serverId` is worth a best-effort Subsonic call while the browser is
|
||||
* online. The active profile uses the connection-status probe; other profiles
|
||||
* (e.g. queue playback while browsing another server) attempt optimistically.
|
||||
*/
|
||||
export function isSubsonicServerReachable(serverId: string): boolean {
|
||||
if (!serverId) return false;
|
||||
if (isDevOfflineBrowseForced()) return false;
|
||||
if (typeof navigator !== 'undefined' && !navigator.onLine) return false;
|
||||
const activeId = useAuthStore.getState().activeServerId;
|
||||
if (!activeId || isSameServerProfile(serverId, activeId)) {
|
||||
return isActiveServerReachable();
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a Subsonic API call for `serverId` is worth attempting.
|
||||
* Skips when the browser or target server is down, or when the track already
|
||||
* plays from a local `psysonic-local://` URL (offline / favorite-auto bytes).
|
||||
*/
|
||||
export function shouldAttemptSubsonicForServer(serverId: string, trackId?: string): boolean {
|
||||
if (!serverId) return false;
|
||||
if (isDevOfflineBrowseForced()) return false;
|
||||
if (typeof navigator !== 'undefined' && !navigator.onLine) return false;
|
||||
if (trackId && hasLocalPlaybackUrl(trackId, serverId)) return false;
|
||||
return isSubsonicServerReachable(serverId);
|
||||
}
|
||||
|
||||
/** Convenience for call sites that only know the active server id. */
|
||||
export function shouldAttemptSubsonicForActiveServer(): boolean {
|
||||
const activeId = useAuthStore.getState().activeServerId;
|
||||
return activeId ? shouldAttemptSubsonicForServer(activeId) : false;
|
||||
}
|
||||
Reference in New Issue
Block a user