From 31542c9923d4dec9a6dc24ff8576cd77839293a5 Mon Sep 17 00:00:00 2001 From: Frank Stellmacher <171614930+Psychotoxical@users.noreply.github.com> Date: Wed, 13 May 2026 14:57:15 +0200 Subject: [PATCH] =?UTF-8?q?refactor(device-sync):=20G.74=20=E2=80=94=20ext?= =?UTF-8?q?ract=20helpers=20+=20legacy=20template=20+=20fetcher=20+=20Brow?= =?UTF-8?q?serRow=20(cluster)=20(#641)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four-cut cluster opening the DeviceSync refactor. 1289 → 1186 LOC (−103). deviceSyncHelpers — uuid, formatBytes, trackToSyncInfo (with the albumArtist-fallback-to-artist logic and the optional playlist context the Rust sync command consumes), plus the SourceTab / SyncStatus / RemovableDrive / SyncTrackMaybePlaylist types. deviceSyncLegacyTemplate — sanitizeComponent (matches Rust's sanitize_path_component) + OldTemplateTrack + applyLegacyTemplate. Lives apart from the general helpers because it's only used by the migration-preview flow and pulls in IS_WINDOWS for path separator. fetchTracksForSource — single async helper that loads the songs for a DeviceSyncSource (playlist / album / artist). Artist sources fan out into parallel getAlbum requests (Navidrome handles them concurrently; the old sequential loop was a ~7 s blocker on 50-album artists). BrowserRow — small button row component used by the source-picker list (playlists / albums / artists tabs). DeviceSync drops the inline definitions plus the direct getAlbum / getPlaylist / getArtist imports that only fetchTracksForSource needed. Pure code move — no behaviour change. --- src/components/deviceSync/BrowserRow.tsx | 24 +++++ src/pages/DeviceSync.tsx | 128 ++--------------------- src/utils/deviceSyncHelpers.ts | 52 +++++++++ src/utils/deviceSyncLegacyTemplate.ts | 35 +++++++ src/utils/fetchTracksForSource.ts | 18 ++++ 5 files changed, 138 insertions(+), 119 deletions(-) create mode 100644 src/components/deviceSync/BrowserRow.tsx create mode 100644 src/utils/deviceSyncHelpers.ts create mode 100644 src/utils/deviceSyncLegacyTemplate.ts create mode 100644 src/utils/fetchTracksForSource.ts diff --git a/src/components/deviceSync/BrowserRow.tsx b/src/components/deviceSync/BrowserRow.tsx new file mode 100644 index 00000000..6dd2ab43 --- /dev/null +++ b/src/components/deviceSync/BrowserRow.tsx @@ -0,0 +1,24 @@ +import React from 'react'; +import { CheckCircle2 } from 'lucide-react'; + +interface Props { + name: string; + meta?: string; + selected: boolean; + onToggle: () => void; + indent?: boolean; +} + +export default function BrowserRow({ name, meta, selected, onToggle, indent }: Props) { + return ( + + ); +} diff --git a/src/pages/DeviceSync.tsx b/src/pages/DeviceSync.tsx index a576311a..40f97460 100644 --- a/src/pages/DeviceSync.tsx +++ b/src/pages/DeviceSync.tsx @@ -1,7 +1,7 @@ -import { getPlaylists, getPlaylist } from '../api/subsonicPlaylists'; +import { getPlaylists } from '../api/subsonicPlaylists'; import { buildDownloadUrl } from '../api/subsonicStreamUrl'; import { getArtists, getArtist } from '../api/subsonicArtists'; -import { getAlbumList, getAlbum } from '../api/subsonicLibrary'; +import { getAlbumList } from '../api/subsonicLibrary'; import type { SubsonicSong, SubsonicAlbum, SubsonicPlaylist, SubsonicArtist } from '../api/subsonicTypes'; import React, { useEffect, useState, useRef, useCallback, useMemo } from 'react'; import { invoke } from '@tauri-apps/api/core'; @@ -20,105 +20,13 @@ import { search as searchSubsonic } from '../api/subsonicSearch'; import { showToast } from '../utils/toast'; import { IS_WINDOWS } from '../utils/platform'; -type SourceTab = 'playlists' | 'albums' | 'artists'; - -// ─── helpers ───────────────────────────────────────────────────────────────── - -function uuid(): string { return crypto.randomUUID(); } - -// Same sanitize rules the Rust side uses (`sanitize_path_component`): strip -// Windows-illegal chars and control chars, trim leading/trailing dots + spaces. -// Kept in JS only for the migration flow — computes the *old* path under a -// user-supplied template so we can diff against the current files on disk. -function sanitizeComponent(s: string): string { - // eslint-disable-next-line no-control-regex - return s.replace(/[/\\:*?"<>|\x00-\x1f\x7f]/g, '_').replace(/^[. ]+|[. ]+$/g, ''); -} - -interface OldTemplateTrack { - artist: string; - album: string; - title: string; - trackNumber?: number; - discNumber?: number; - year?: number; - suffix: string; -} - -/** Renders a track's path under a legacy (user-configurable) template. Used only - * for the migration preview — the live sync flow goes through Rust's fixed - * `build_track_path`. */ -function applyLegacyTemplate(template: string, track: OldTemplateTrack): string { - const relative = template - .replace(/\{artist\}/g, sanitizeComponent(track.artist)) - .replace(/\{album\}/g, sanitizeComponent(track.album)) - .replace(/\{title\}/g, sanitizeComponent(track.title)) - .replace(/\{track_number\}/g, track.trackNumber != null ? String(track.trackNumber).padStart(2, '0') : '') - .replace(/\{disc_number\}/g, track.discNumber != null ? String(track.discNumber) : '') - .replace(/\{year\}/g, track.year != null ? String(track.year) : ''); - const withExt = `${relative}.${track.suffix}`; - return IS_WINDOWS ? withExt.replace(/\//g, '\\') : withExt; -} - -async function fetchTracksForSource(source: DeviceSyncSource): Promise { - if (source.type === 'playlist') { const { songs } = await getPlaylist(source.id); return songs; } - if (source.type === 'album') { const { songs } = await getAlbum(source.id); return songs; } - const { albums } = await getArtist(source.id); - // Parallel album fetches — Navidrome handles getAlbum requests in flight - // without serialising. Sequential awaits here multiplied a 50-album artist - // sync into 50 round-trips (~7 s blocking) before any device write started. - const results = await Promise.all( - albums.map(a => getAlbum(a.id).then(r => r.songs).catch(() => [] as SubsonicSong[])), - ); - return results.flat(); -} - -/** Tracks that came from `calculate_sync_payload` may carry embedded playlist - * context so the follow-up `sync_batch_to_device` call knows to place them - * under `Playlists/{Name}/` instead of the album tree. */ -type SyncTrackMaybePlaylist = SubsonicSong & { _playlistName?: string; _playlistIndex?: number }; - -function trackToSyncInfo( - track: SyncTrackMaybePlaylist, - url: string, - playlistCtx?: { name: string; index: number }, -) { - // Fall back to track artist when the file has no albumArtist tag — not every - // library is tagged with it. Treat empty strings as missing (some Subsonic - // servers return "" rather than omitting the field). - const albumArtist = (track.albumArtist?.trim() || track.artist?.trim() || ''); - return { - id: track.id, url, - suffix: track.suffix ?? 'mp3', - artist: track.artist ?? '', - albumArtist, - album: track.album ?? '', - title: track.title ?? '', - trackNumber: track.track, - duration: track.duration, - playlistName: playlistCtx?.name ?? track._playlistName, - playlistIndex: playlistCtx?.index ?? track._playlistIndex, - }; -} - -type SyncStatus = 'synced' | 'pending' | 'deletion'; - -interface RemovableDrive { - name: string; - mount_point: string; - available_space: number; - total_space: number; - file_system: string; - is_removable: boolean; -} - -function formatBytes(bytes: number): string { - if (bytes === 0) return '0 B'; - const k = 1024; - const sizes = ['B', 'KB', 'MB', 'GB', 'TB']; - const i = Math.floor(Math.log(bytes) / Math.log(k)); - return `${(bytes / Math.pow(k, i)).toFixed(1)} ${sizes[i]}`; -} +import { + uuid, formatBytes, trackToSyncInfo, + type SourceTab, type SyncStatus, type RemovableDrive, +} from '../utils/deviceSyncHelpers'; +import { applyLegacyTemplate } from '../utils/deviceSyncLegacyTemplate'; +import { fetchTracksForSource } from '../utils/fetchTracksForSource'; +import BrowserRow from '../components/deviceSync/BrowserRow'; // ─── component ─────────────────────────────────────────────────────────────── @@ -1269,21 +1177,3 @@ export default function DeviceSync() { ); } - -// ─── BrowserRow ────────────────────────────────────────────────────────────── - -function BrowserRow({ name, meta, selected, onToggle, indent }: { - name: string; meta?: string; selected: boolean; onToggle: () => void; indent?: boolean; -}) { - return ( - - ); -} diff --git a/src/utils/deviceSyncHelpers.ts b/src/utils/deviceSyncHelpers.ts new file mode 100644 index 00000000..3d6ebf79 --- /dev/null +++ b/src/utils/deviceSyncHelpers.ts @@ -0,0 +1,52 @@ +import type { SubsonicSong } from '../api/subsonicTypes'; + +export type SourceTab = 'playlists' | 'albums' | 'artists'; + +export function uuid(): string { return crypto.randomUUID(); } + +export type SyncStatus = 'synced' | 'pending' | 'deletion'; + +export interface RemovableDrive { + name: string; + mount_point: string; + available_space: number; + total_space: number; + file_system: string; + is_removable: boolean; +} + +export function formatBytes(bytes: number): string { + if (bytes === 0) return '0 B'; + const k = 1024; + const sizes = ['B', 'KB', 'MB', 'GB', 'TB']; + const i = Math.floor(Math.log(bytes) / Math.log(k)); + return `${(bytes / Math.pow(k, i)).toFixed(1)} ${sizes[i]}`; +} + +/** Tracks that came from `calculate_sync_payload` may carry embedded playlist + * context so the follow-up `sync_batch_to_device` call knows to place them + * under `Playlists/{Name}/` instead of the album tree. */ +export type SyncTrackMaybePlaylist = SubsonicSong & { _playlistName?: string; _playlistIndex?: number }; + +export function trackToSyncInfo( + track: SyncTrackMaybePlaylist, + url: string, + playlistCtx?: { name: string; index: number }, +) { + // Fall back to track artist when the file has no albumArtist tag — not every + // library is tagged with it. Treat empty strings as missing (some Subsonic + // servers return "" rather than omitting the field). + const albumArtist = (track.albumArtist?.trim() || track.artist?.trim() || ''); + return { + id: track.id, url, + suffix: track.suffix ?? 'mp3', + artist: track.artist ?? '', + albumArtist, + album: track.album ?? '', + title: track.title ?? '', + trackNumber: track.track, + duration: track.duration, + playlistName: playlistCtx?.name ?? track._playlistName, + playlistIndex: playlistCtx?.index ?? track._playlistIndex, + }; +} diff --git a/src/utils/deviceSyncLegacyTemplate.ts b/src/utils/deviceSyncLegacyTemplate.ts new file mode 100644 index 00000000..3037653e --- /dev/null +++ b/src/utils/deviceSyncLegacyTemplate.ts @@ -0,0 +1,35 @@ +import { IS_WINDOWS } from './platform'; + +// Same sanitize rules the Rust side uses (`sanitize_path_component`): strip +// Windows-illegal chars and control chars, trim leading/trailing dots + spaces. +// Kept in JS only for the migration flow — computes the *old* path under a +// user-supplied template so we can diff against the current files on disk. +export function sanitizeComponent(s: string): string { + // eslint-disable-next-line no-control-regex + return s.replace(/[/\\:*?"<>|\x00-\x1f\x7f]/g, '_').replace(/^[. ]+|[. ]+$/g, ''); +} + +export interface OldTemplateTrack { + artist: string; + album: string; + title: string; + trackNumber?: number; + discNumber?: number; + year?: number; + suffix: string; +} + +/** Renders a track's path under a legacy (user-configurable) template. Used only + * for the migration preview — the live sync flow goes through Rust's fixed + * `build_track_path`. */ +export function applyLegacyTemplate(template: string, track: OldTemplateTrack): string { + const relative = template + .replace(/\{artist\}/g, sanitizeComponent(track.artist)) + .replace(/\{album\}/g, sanitizeComponent(track.album)) + .replace(/\{title\}/g, sanitizeComponent(track.title)) + .replace(/\{track_number\}/g, track.trackNumber != null ? String(track.trackNumber).padStart(2, '0') : '') + .replace(/\{disc_number\}/g, track.discNumber != null ? String(track.discNumber) : '') + .replace(/\{year\}/g, track.year != null ? String(track.year) : ''); + const withExt = `${relative}.${track.suffix}`; + return IS_WINDOWS ? withExt.replace(/\//g, '\\') : withExt; +} diff --git a/src/utils/fetchTracksForSource.ts b/src/utils/fetchTracksForSource.ts new file mode 100644 index 00000000..aaff8aab --- /dev/null +++ b/src/utils/fetchTracksForSource.ts @@ -0,0 +1,18 @@ +import { getArtist } from '../api/subsonicArtists'; +import { getAlbum } from '../api/subsonicLibrary'; +import { getPlaylist } from '../api/subsonicPlaylists'; +import type { SubsonicSong } from '../api/subsonicTypes'; +import type { DeviceSyncSource } from '../store/deviceSyncStore'; + +export async function fetchTracksForSource(source: DeviceSyncSource): Promise { + if (source.type === 'playlist') { const { songs } = await getPlaylist(source.id); return songs; } + if (source.type === 'album') { const { songs } = await getAlbum(source.id); return songs; } + const { albums } = await getArtist(source.id); + // Parallel album fetches — Navidrome handles getAlbum requests in flight + // without serialising. Sequential awaits here multiplied a 50-album artist + // sync into 50 round-trips (~7 s blocking) before any device write started. + const results = await Promise.all( + albums.map(a => getAlbum(a.id).then(r => r.songs).catch(() => [] as SubsonicSong[])), + ); + return results.flat(); +}