From 6880cffd16b5016592ab95a44bd01d7fc7d1b8a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Wed, 21 May 2025 09:30:23 -0400 Subject: [PATCH] feat(ui): add scan progress and error reporting to UI (#4094) * feat(scanner): add LastScanError tracking to scanner status - Introduced LastScanErrorKey constant for error tracking. - Updated StatusInfo struct to include LastError field. - Modified scanner logic to store and retrieve last scan error. - Enhanced ScanStatus response to include error information. - Updated UI components to display last scan error when applicable. - Added tests to verify last scan error functionality. Signed-off-by: Deluan * feat(scanner): enhance scan status with type and elapsed time tracking - Added LastScanTypeKey and LastScanStartTimeKey constants for tracking scan type and start time. - Updated StatusInfo struct to include ScanType and ElapsedTime fields. - Implemented getScanInfo method to retrieve scan type, elapsed time, and last error. - Modified scanner logic to store scan type and start time during scans. - Enhanced ScanStatus response and UI components to display scan type and elapsed time. - Added formatShortDuration utility for better elapsed time representation. - Updated activity reducer to handle new scan status fields. Signed-off-by: Deluan * refactor(tests): consolidate controller status tests into a single file - Removed the old controller_status_test.go file. - Merged relevant tests into the new controller_test.go file for better organization and maintainability. - Ensured all existing test cases for controller status are preserved and functional. Signed-off-by: Deluan * Fix formatting issues * refactor(scanner): update getScanInfo method documentation --------- Signed-off-by: Deluan --- consts/consts.go | 3 + scanner/controller.go | 47 ++++++++++ scanner/controller_test.go | 57 ++++++++++++ scanner/scanner.go | 12 +++ server/events/events.go | 9 +- server/subsonic/library_scanning.go | 3 + server/subsonic/responses/responses.go | 11 ++- ui/src/i18n/en.json | 5 +- ui/src/layout/ActivityPanel.jsx | 81 ++++++++++++++-- ui/src/reducers/activityReducer.js | 15 ++- ui/src/reducers/activityReducer.test.js | 119 ++++++++++++++++++++++++ ui/src/utils/formatters.js | 20 ++++ ui/src/utils/formatters.test.js | 34 ++++++- 13 files changed, 394 insertions(+), 22 deletions(-) create mode 100644 scanner/controller_test.go create mode 100644 ui/src/reducers/activityReducer.test.js diff --git a/consts/consts.go b/consts/consts.go index 75271bec8..2dbb46d07 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -14,6 +14,9 @@ const ( DefaultDbPath = "navidrome.db?cache=shared&_busy_timeout=15000&_journal_mode=WAL&_foreign_keys=on&synchronous=normal" InitialSetupFlagKey = "InitialSetup" FullScanAfterMigrationFlagKey = "FullScanAfterMigration" + LastScanErrorKey = "LastScanError" + LastScanTypeKey = "LastScanType" + LastScanStartTimeKey = "LastScanStartTime" UIAuthorizationHeader = "X-ND-Authorization" UIClientUniqueIDHeader = "X-ND-Client-Unique-Id" diff --git a/scanner/controller.go b/scanner/controller.go index e3e008483..36048e6f3 100644 --- a/scanner/controller.go +++ b/scanner/controller.go @@ -9,6 +9,7 @@ import ( "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/core/auth" @@ -37,6 +38,9 @@ type StatusInfo struct { LastScan time.Time Count uint32 FolderCount uint32 + LastError string + ScanType string + ElapsedTime time.Duration } func New(rootCtx context.Context, ds model.DataStore, cw artwork.CacheWarmer, broker events.Broker, @@ -113,20 +117,51 @@ type controller struct { changesDetected bool } +// getScanInfo retrieves scan status from the database +func (s *controller) getScanInfo(ctx context.Context) (scanType string, elapsed time.Duration, lastErr string) { + lastErr, _ = s.ds.Property(ctx).DefaultGet(consts.LastScanErrorKey, "") + scanType, _ = s.ds.Property(ctx).DefaultGet(consts.LastScanTypeKey, "") + startTimeStr, _ := s.ds.Property(ctx).DefaultGet(consts.LastScanStartTimeKey, "") + + if startTimeStr != "" { + startTime, err := time.Parse(time.RFC3339, startTimeStr) + if err == nil { + if running.Load() { + elapsed = time.Since(startTime) + } else { + // If scan is not running, try to get the last scan time for the library + lib, err := s.ds.Library(ctx).Get(1) //TODO Multi-library + if err == nil { + elapsed = lib.LastScanAt.Sub(startTime) + } + } + } + } + + return scanType, elapsed, lastErr +} + func (s *controller) Status(ctx context.Context) (*StatusInfo, error) { lib, err := s.ds.Library(ctx).Get(1) //TODO Multi-library if err != nil { return nil, fmt.Errorf("getting library: %w", err) } + + scanType, elapsed, lastErr := s.getScanInfo(ctx) + if running.Load() { status := &StatusInfo{ Scanning: true, LastScan: lib.LastScanAt, Count: s.count.Load(), FolderCount: s.folderCount.Load(), + LastError: lastErr, + ScanType: scanType, + ElapsedTime: elapsed, } return status, nil } + count, folderCount, err := s.getCounters(ctx) if err != nil { return nil, fmt.Errorf("getting library stats: %w", err) @@ -136,6 +171,9 @@ func (s *controller) Status(ctx context.Context) (*StatusInfo, error) { LastScan: lib.LastScanAt, Count: uint32(count), FolderCount: uint32(folderCount), + LastError: lastErr, + ScanType: scanType, + ElapsedTime: elapsed, }, nil } @@ -193,10 +231,14 @@ func (s *controller) ScanAll(requestCtx context.Context, fullScan bool) ([]strin if count, folderCount, err := s.getCounters(ctx); err != nil { return scanWarnings, err } else { + scanType, elapsed, lastErr := s.getScanInfo(ctx) s.sendMessage(ctx, &events.ScanStatus{ Scanning: false, Count: count, FolderCount: folderCount, + Error: lastErr, + ScanType: scanType, + ElapsedTime: elapsed, }) } return scanWarnings, scanError @@ -240,10 +282,15 @@ func (s *controller) trackProgress(ctx context.Context, progress <-chan *Progres if p.FileCount > 0 { s.folderCount.Add(1) } + + scanType, elapsed, lastErr := s.getScanInfo(ctx) status := &events.ScanStatus{ Scanning: true, Count: int64(s.count.Load()), FolderCount: int64(s.folderCount.Load()), + Error: lastErr, + ScanType: scanType, + ElapsedTime: elapsed, } if s.limiter != nil { s.limiter.Do(func() { s.sendMessage(ctx, status) }) diff --git a/scanner/controller_test.go b/scanner/controller_test.go new file mode 100644 index 000000000..4f6576a39 --- /dev/null +++ b/scanner/controller_test.go @@ -0,0 +1,57 @@ +package scanner_test + +import ( + "context" + + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/core" + "github.com/navidrome/navidrome/core/artwork" + "github.com/navidrome/navidrome/core/metrics" + "github.com/navidrome/navidrome/db" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/persistence" + "github.com/navidrome/navidrome/scanner" + "github.com/navidrome/navidrome/server/events" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Controller", func() { + var ctx context.Context + var ds *tests.MockDataStore + var ctrl scanner.Scanner + + Describe("Status", func() { + BeforeEach(func() { + ctx = context.Background() + db.Init(ctx) + DeferCleanup(func() { Expect(tests.ClearDB()).To(Succeed()) }) + DeferCleanup(configtest.SetupConfig()) + ds = &tests.MockDataStore{RealDS: persistence.New(db.Db())} + ds.MockedProperty = &tests.MockedPropertyRepo{} + ctrl = scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), core.NewPlaylists(ds), metrics.NewNoopInstance()) + Expect(ds.Library(ctx).Put(&model.Library{ID: 1, Name: "lib", Path: "/tmp"})).To(Succeed()) + }) + + It("includes last scan error", func() { + Expect(ds.Property(ctx).Put(consts.LastScanErrorKey, "boom")).To(Succeed()) + status, err := ctrl.Status(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(status.LastError).To(Equal("boom")) + }) + + It("includes scan type and error in status", func() { + // Set up test data in property repo + Expect(ds.Property(ctx).Put(consts.LastScanErrorKey, "test error")).To(Succeed()) + Expect(ds.Property(ctx).Put(consts.LastScanTypeKey, "full")).To(Succeed()) + + // Get status and verify basic info + status, err := ctrl.Status(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(status.LastError).To(Equal("test error")) + Expect(status.ScanType).To(Equal("full")) + }) + }) +}) diff --git a/scanner/scanner.go b/scanner/scanner.go index 1c08e3fb3..21de5f956 100644 --- a/scanner/scanner.go +++ b/scanner/scanner.go @@ -57,12 +57,21 @@ func (s *scannerImpl) scanAll(ctx context.Context, fullScan bool, progress chan< startTime := time.Now() log.Info(ctx, "Scanner: Starting scan", "fullScan", state.fullScan, "numLibraries", len(libs)) + // Store scan type and start time + scanType := "quick" + if state.fullScan { + scanType = "full" + } + _ = s.ds.Property(ctx).Put(consts.LastScanTypeKey, scanType) + _ = s.ds.Property(ctx).Put(consts.LastScanStartTimeKey, startTime.Format(time.RFC3339)) + // if there was a full scan in progress, force a full scan if !state.fullScan { for _, lib := range libs { if lib.FullScanInProgress { log.Info(ctx, "Scanner: Interrupted full scan detected", "lib", lib.Name) state.fullScan = true + _ = s.ds.Property(ctx).Put(consts.LastScanTypeKey, "full") break } } @@ -100,11 +109,14 @@ func (s *scannerImpl) scanAll(ctx context.Context, fullScan bool, progress chan< ) if err != nil { log.Error(ctx, "Scanner: Finished with error", "duration", time.Since(startTime), err) + _ = s.ds.Property(ctx).Put(consts.LastScanErrorKey, err.Error()) state.sendError(err) s.metrics.WriteAfterScanMetrics(ctx, false) return } + _ = s.ds.Property(ctx).Put(consts.LastScanErrorKey, "") + if state.changesDetected.Load() { state.sendProgress(&ProgressInfo{ChangesDetected: true}) } diff --git a/server/events/events.go b/server/events/events.go index 38b906f2a..73ff8eb5e 100644 --- a/server/events/events.go +++ b/server/events/events.go @@ -37,9 +37,12 @@ func (e *baseEvent) Data(evt Event) string { type ScanStatus struct { baseEvent - Scanning bool `json:"scanning"` - Count int64 `json:"count"` - FolderCount int64 `json:"folderCount"` + Scanning bool `json:"scanning"` + Count int64 `json:"count"` + FolderCount int64 `json:"folderCount"` + Error string `json:"error"` + ScanType string `json:"scanType"` + ElapsedTime time.Duration `json:"elapsedTime"` } type KeepAlive struct { diff --git a/server/subsonic/library_scanning.go b/server/subsonic/library_scanning.go index a25955ea7..b6ccb9ae6 100644 --- a/server/subsonic/library_scanning.go +++ b/server/subsonic/library_scanning.go @@ -23,6 +23,9 @@ func (api *Router) GetScanStatus(r *http.Request) (*responses.Subsonic, error) { Count: int64(status.Count), FolderCount: int64(status.FolderCount), LastScan: &status.LastScan, + Error: status.LastError, + ScanType: status.ScanType, + ElapsedTime: int64(status.ElapsedTime), } return response, nil } diff --git a/server/subsonic/responses/responses.go b/server/subsonic/responses/responses.go index e886d6563..4a7ebbe83 100644 --- a/server/subsonic/responses/responses.go +++ b/server/subsonic/responses/responses.go @@ -477,10 +477,13 @@ type Shares struct { } type ScanStatus struct { - Scanning bool `xml:"scanning,attr" json:"scanning"` - Count int64 `xml:"count,attr" json:"count"` - FolderCount int64 `xml:"folderCount,attr" json:"folderCount"` - LastScan *time.Time `xml:"lastScan,attr,omitempty" json:"lastScan,omitempty"` + Scanning bool `xml:"scanning,attr" json:"scanning"` + Count int64 `xml:"count,attr" json:"count"` + FolderCount int64 `xml:"folderCount,attr" json:"folderCount"` + LastScan *time.Time `xml:"lastScan,attr,omitempty" json:"lastScan,omitempty"` + Error string `xml:"error,attr,omitempty" json:"error,omitempty"` + ScanType string `xml:"scanType,attr,omitempty" json:"scanType,omitempty"` + ElapsedTime int64 `xml:"elapsedTime,attr,omitempty" json:"elapsedTime,omitempty"` } type Lyrics struct { diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index 76c4d5190..5ccebf115 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -499,7 +499,10 @@ "quickScan": "Quick Scan", "fullScan": "Full Scan", "serverUptime": "Server Uptime", - "serverDown": "OFFLINE" + "serverDown": "OFFLINE", + "scanType": "Type", + "status": "Scan Error", + "elapsedTime": "Elapsed Time" }, "help": { "title": "Navidrome Hotkeys", diff --git a/ui/src/layout/ActivityPanel.jsx b/ui/src/layout/ActivityPanel.jsx index 840c54b6f..f7371bc2f 100644 --- a/ui/src/layout/ActivityPanel.jsx +++ b/ui/src/layout/ActivityPanel.jsx @@ -12,15 +12,16 @@ import { CardActions, Divider, Box, + Typography, } from '@material-ui/core' import { FiActivity } from 'react-icons/fi' -import { BiError } from 'react-icons/bi' +import { BiError, BiCheckCircle } from 'react-icons/bi' import { VscSync } from 'react-icons/vsc' import { GiMagnifyingGlass } from 'react-icons/gi' import subsonic from '../subsonic' import { scanStatusUpdate } from '../actions' import { useInterval } from '../common' -import { formatDuration } from '../utils' +import { formatDuration, formatShortDuration } from '../utils' import config from '../config' const useStyles = makeStyles((theme) => ({ @@ -40,7 +41,16 @@ const useStyles = makeStyles((theme) => ({ zIndex: 2, }, counterStatus: { - minWidth: '15em', + minWidth: '20em', + }, + error: { + color: theme.palette.error.main, + }, + card: { + maxWidth: 'none', + }, + cardContent: { + padding: theme.spacing(2, 3), }, })) @@ -59,13 +69,13 @@ const Uptime = () => { const ActivityPanel = () => { const serverStart = useSelector((state) => state.activity.serverStart) const up = serverStart.startTime - const classes = useStyles({ up }) + const scanStatus = useSelector((state) => state.activity.scanStatus) + const classes = useStyles({ up: up && !scanStatus.error }) const translate = useTranslate() const notify = useNotify() const [anchorEl, setAnchorEl] = useState(null) const open = Boolean(anchorEl) const dispatch = useDispatch() - const scanStatus = useSelector((state) => state.activity.scanStatus) const handleMenuOpen = (event) => setAnchorEl(event.currentTarget) const handleMenuClose = () => setAnchorEl(null) @@ -89,11 +99,30 @@ const ActivityPanel = () => { } }, [serverStart, notify]) + const tooltipTitle = scanStatus.error + ? `${translate('activity.status')}: ${scanStatus.error}` + : translate('activity.title') + + const lastScanType = (() => { + switch (scanStatus.scanType) { + case 'full': + return translate('activity.fullScan') + case 'quick': + return translate('activity.quickScan') + default: + return '' + } + })() + return (
- + - {up ? : } + {!up || scanStatus.error ? ( + + ) : ( + + )} {scanStatus.scanning && ( @@ -113,8 +142,8 @@ const ActivityPanel = () => { open={open} onClose={handleMenuClose} > - - + + {translate('activity.serverUptime')}: @@ -125,7 +154,7 @@ const ActivityPanel = () => { - + {translate('activity.totalScanned')}: @@ -134,6 +163,38 @@ const ActivityPanel = () => { {scanStatus.folderCount || '-'} + + + + {translate('activity.scanType')}: + + + {lastScanType} + + + + + + {translate('activity.elapsedTime')}: + + + {formatShortDuration(scanStatus.elapsedTime)} + + + + {scanStatus.error && ( + + + {translate('activity.status')}: + + {scanStatus.error} + + )} diff --git a/ui/src/reducers/activityReducer.js b/ui/src/reducers/activityReducer.js index d2a7d310b..2b6d2741c 100644 --- a/ui/src/reducers/activityReducer.js +++ b/ui/src/reducers/activityReducer.js @@ -6,15 +6,24 @@ import { import config from '../config' const initialState = { - scanStatus: { scanning: false, folderCount: 0, count: 0 }, + scanStatus: { + scanning: false, + folderCount: 0, + count: 0, + error: '', + elapsedTime: 0, + }, serverStart: { version: config.version }, } export const activityReducer = (previousState = initialState, payload) => { const { type, data } = payload + switch (type) { - case EVENT_SCAN_STATUS: - return { ...previousState, scanStatus: data } + case EVENT_SCAN_STATUS: { + const elapsedTime = Number(data.elapsedTime) || 0 + return { ...previousState, scanStatus: { ...data, elapsedTime } } + } case EVENT_SERVER_START: return { ...previousState, diff --git a/ui/src/reducers/activityReducer.test.js b/ui/src/reducers/activityReducer.test.js new file mode 100644 index 000000000..a1389e3d2 --- /dev/null +++ b/ui/src/reducers/activityReducer.test.js @@ -0,0 +1,119 @@ +import { activityReducer } from './activityReducer' +import { EVENT_SCAN_STATUS, EVENT_SERVER_START } from '../actions' +import config from '../config' + +describe('activityReducer', () => { + const initialState = { + scanStatus: { + scanning: false, + folderCount: 0, + count: 0, + error: '', + elapsedTime: 0, + }, + serverStart: { version: config.version }, + } + + it('returns the initial state when no action is specified', () => { + expect(activityReducer(undefined, {})).toEqual(initialState) + }) + + it('handles EVENT_SCAN_STATUS action with elapsedTime field', () => { + const elapsedTime = 123456789 // nanoseconds + const action = { + type: EVENT_SCAN_STATUS, + data: { + scanning: true, + folderCount: 5, + count: 100, + error: '', + elapsedTime: elapsedTime, + }, + } + + const newState = activityReducer(initialState, action) + expect(newState.scanStatus).toEqual({ + scanning: true, + folderCount: 5, + count: 100, + error: '', + elapsedTime: elapsedTime, + }) + }) + + it('handles EVENT_SCAN_STATUS action with string elapsedTime', () => { + const action = { + type: EVENT_SCAN_STATUS, + data: { + scanning: true, + folderCount: 5, + count: 100, + error: '', + elapsedTime: '123456789', + }, + } + + const newState = activityReducer(initialState, action) + expect(newState.scanStatus.elapsedTime).toEqual(123456789) + }) + + it('handles EVENT_SCAN_STATUS with error field', () => { + const action = { + type: EVENT_SCAN_STATUS, + data: { + scanning: false, + folderCount: 0, + count: 0, + error: 'Test error message', + elapsedTime: 0, + }, + } + + const newState = activityReducer(initialState, action) + expect(newState.scanStatus.error).toEqual('Test error message') + }) + + it('handles EVENT_SERVER_START action', () => { + const action = { + type: EVENT_SERVER_START, + data: { + version: '1.0.0', + startTime: '2023-01-01T00:00:00Z', + }, + } + + const newState = activityReducer(initialState, action) + expect(newState.serverStart).toEqual({ + version: '1.0.0', + startTime: Date.parse('2023-01-01T00:00:00Z'), + }) + }) + + it('preserves the scanStatus when handling EVENT_SERVER_START', () => { + const currentState = { + scanStatus: { + scanning: true, + folderCount: 5, + count: 100, + error: 'Previous error', + elapsedTime: 12345, + }, + serverStart: { version: config.version }, + } + + const action = { + type: EVENT_SERVER_START, + data: { + version: '1.0.0', + startTime: '2023-01-01T00:00:00Z', + }, + } + + const newState = activityReducer(currentState, action) + expect(newState.scanStatus).toEqual(currentState.scanStatus) + expect(newState.serverStart).toEqual({ + version: '1.0.0', + startTime: Date.parse('2023-01-01T00:00:00Z'), + }) + }) +}) diff --git a/ui/src/utils/formatters.js b/ui/src/utils/formatters.js index 25daa33d4..ae27f230f 100644 --- a/ui/src/utils/formatters.js +++ b/ui/src/utils/formatters.js @@ -25,6 +25,26 @@ export const formatDuration = (d) => { return `${days > 0 ? days + ':' : ''}${f}` } +export const formatShortDuration = (ns) => { + // Convert nanoseconds to seconds + const seconds = ns / 1e9 + if (seconds < 1.0) { + return '<1s' + } + + const hours = Math.floor(seconds / 3600) + const minutes = Math.floor((seconds % 3600) / 60) + const secs = Math.floor(seconds % 60) + + if (hours > 0) { + return `${hours}h${minutes}m` + } + if (minutes > 0) { + return `${minutes}m${secs}s` + } + return `${secs}s` +} + export const formatFullDate = (date, locale) => { const dashes = date.split('-').length - 1 let options = { diff --git a/ui/src/utils/formatters.test.js b/ui/src/utils/formatters.test.js index 59538ec32..87b40f16b 100644 --- a/ui/src/utils/formatters.test.js +++ b/ui/src/utils/formatters.test.js @@ -1,4 +1,9 @@ -import { formatBytes, formatDuration, formatFullDate } from './formatters' +import { + formatBytes, + formatDuration, + formatFullDate, + formatShortDuration, +} from './formatters' describe('formatBytes', () => { it('format bytes', () => { @@ -32,6 +37,33 @@ describe('formatDuration', () => { }) }) +describe('formatShortDuration', () => { + // Convert seconds to nanoseconds for the tests + const toNs = (seconds) => seconds * 1e9 + + it('formats less than a second', () => { + expect(formatShortDuration(toNs(0.5))).toEqual('<1s') + expect(formatShortDuration(toNs(0))).toEqual('<1s') + }) + + it('formats seconds', () => { + expect(formatShortDuration(toNs(1))).toEqual('1s') + expect(formatShortDuration(toNs(59))).toEqual('59s') + }) + + it('formats minutes and seconds', () => { + expect(formatShortDuration(toNs(60))).toEqual('1m0s') + expect(formatShortDuration(toNs(90))).toEqual('1m30s') + expect(formatShortDuration(toNs(59 * 60 + 59))).toEqual('59m59s') + }) + + it('formats hours and minutes', () => { + expect(formatShortDuration(toNs(3600))).toEqual('1h0m') + expect(formatShortDuration(toNs(3600 + 30 * 60))).toEqual('1h30m') + expect(formatShortDuration(toNs(24 * 3600 - 1))).toEqual('23h59m') + }) +}) + describe('formatFullDate', () => { it('format dates', () => { expect(formatFullDate('2011', 'en-US')).toEqual('2011')