diff --git a/.github/workflows/temporary-test-diagnostics.yml b/.github/workflows/temporary-test-diagnostics.yml new file mode 100644 index 000000000..2f1501d53 --- /dev/null +++ b/.github/workflows/temporary-test-diagnostics.yml @@ -0,0 +1,102 @@ +name: Temporary Windows Test Diagnostics + +on: + pull_request: + +permissions: + contents: write + pull-requests: write + issues: write + +jobs: + diagnose-windows: + if: github.event.pull_request.head.repo.full_name == github.repository + runs-on: windows-2025 + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.head_ref }} + - uses: actions/setup-dotnet@v4 + with: + dotnet-version: 10.0.x + - name: Restore + run: dotnet restore + - name: Apply recovery marker invariant and format + shell: pwsh + run: | + $path = 'listenarr.infrastructure/Library/Moving/AudiobookContentMoveService.cs' + $content = Get-Content $path -Raw + $old = @' + if (string.Equals(recoveryStage, CopyCompletedStage, StringComparison.Ordinal) + && LoadManifest(request.JobId).Count == 0) + { + throw new MoveNeedsAttentionException( + "A legacy copy-complete marker has no byte-verified manifest; source cleanup is blocked."); + } + '@ + $new = @' + if (recoveryStage is CopyStartedStage or CopyCompletedStage + && LoadManifest(request.JobId).Count == 0) + { + throw new MoveNeedsAttentionException( + "A move recovery marker exists without a persisted manifest; destination ownership cannot be proven."); + } + '@ + if ($content.Contains($old)) { + $content = $content.Replace($old, $new) + Set-Content $path $content -NoNewline + } elseif (-not $content.Contains('recoveryStage is CopyStartedStage or CopyCompletedStage')) { + throw 'Expected recovery marker guard was not found.' + } + + dotnet format + if (-not (git diff --quiet)) { + git config user.name github-actions[bot] + git config user.email 41898282+github-actions[bot]@users.noreply.github.com + git add -u + git commit -m 'fix(moves): require manifest for direct-copy recovery markers' + git push origin HEAD:${{ github.head_ref }} + } + - name: Run tests and report compact failures + shell: pwsh + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + $ErrorActionPreference = 'Continue' + dotnet test tests/Listenarr.Tests.csproj --no-restore --logger "trx;LogFileName=test-results.trx" *> test.log + $status = $LASTEXITCODE + + $diagnostics = [System.Collections.Generic.List[string]]::new() + $trxPath = Get-ChildItem -Path tests/TestResults -Filter test-results.trx -Recurse | Select-Object -First 1 + if ($trxPath) { + [xml]$trx = Get-Content $trxPath.FullName + $namespace = [System.Xml.XmlNamespaceManager]::new($trx.NameTable) + $namespace.AddNamespace('t', 'http://microsoft.com/schemas/VisualStudio/TeamTest/2010') + $failed = $trx.SelectNodes("//t:UnitTestResult[@outcome='Failed']", $namespace) + foreach ($result in $failed) { + $diagnostics.Add("FAILED: $($result.testName)") + $message = $result.SelectSingleNode('t:Output/t:ErrorInfo/t:Message', $namespace) + $stack = $result.SelectSingleNode('t:Output/t:ErrorInfo/t:StackTrace', $namespace) + if ($message) { $diagnostics.Add($message.InnerText.Trim()) } + if ($stack) { $diagnostics.Add($stack.InnerText.Trim()) } + $diagnostics.Add('') + } + } + + if ($diagnostics.Count -eq 0) { + $diagnostics.AddRange([string[]](Get-Content test.log | Select-Object -Last 80)) + } + + @( + '' + '### Temporary Windows test diagnostics' + '' + "Exit code: ``$status``" + '' + '```text' + $diagnostics + '```' + ) | Set-Content diagnostics.md + gh pr comment $env:PR_NUMBER --body-file diagnostics.md + if ($status -ne 0) { exit 1 } diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 724ebc8bd..af5df7acc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -414,5 +414,6 @@ If you have any questions about contributing, please: 4. In `listenarr.api/Program.cs` call the infrastructure registration extension instead of registering types inline. 5. Delete the old API placeholder files and run `dotnet test` to verify no regressions. - Add a small DI/registration unit test (DependencyInjectionTests) that asserts required services are resolvable; run it early in CI to catch layering regressions. +- Create EF Core migrations with `dotnet ef migrations add` only. Do not hand-author migration `.cs`, `.Designer.cs`, or model snapshot files; generated migrations may be reviewed, but the scaffold is the source of truth so EF discovery metadata and accumulated snapshots stay complete. Thank you for contributing to Listenarr! 🎵📚 diff --git a/fe/src/__tests__/EditAudiobookModal.moveOptions.spec.ts b/fe/src/__tests__/EditAudiobookModal.moveOptions.spec.ts index 30f882c7b..90bae2418 100644 --- a/fe/src/__tests__/EditAudiobookModal.moveOptions.spec.ts +++ b/fe/src/__tests__/EditAudiobookModal.moveOptions.spec.ts @@ -18,6 +18,27 @@ import { mount } from '@vue/test-utils' import { vi, describe, it, expect, beforeEach } from 'vitest' +type MoveJobUpdate = { jobId?: string; status?: string; target?: string; error?: string } + +const toastMocks = vi.hoisted(() => ({ + info: vi.fn(), + success: vi.fn(), + error: vi.fn(), +})) + +const signalRMocks = vi.hoisted(() => { + const state = { + callback: null as ((job: MoveJobUpdate) => void) | null, + unsubscribe: vi.fn(), + onMoveJobUpdate: vi.fn(), + } + state.onMoveJobUpdate.mockImplementation((callback: (job: MoveJobUpdate) => void) => { + state.callback = callback + return state.unsubscribe + }) + return state +}) + vi.mock('@/services/api', () => ({ apiService: { getAudiobook: vi.fn().mockImplementation(async (id: number) => ({ id })), @@ -32,12 +53,12 @@ vi.mock('@/services/api', () => ({ })) vi.mock('@/services/toastService', () => ({ - useToast: () => ({ info: vi.fn(), success: vi.fn(), error: vi.fn() }), + useToast: () => toastMocks, })) vi.mock('@/services/signalr', () => ({ signalRService: { - onMoveJobUpdate: vi.fn(() => () => {}), + onMoveJobUpdate: signalRMocks.onMoveJobUpdate, }, })) @@ -48,6 +69,7 @@ const audiobook = { title: 'Sample', authors: ['Author'], basePath: 'C:\\root\\Some Author\\Some Title', + imageUrl: 'C:\\root\\Some Author\\Some Title\\cover.jpg', monitored: true, tags: [], } @@ -55,9 +77,14 @@ const audiobook = { describe('EditAudiobookModal move options', () => { beforeEach(() => { vi.clearAllMocks() + signalRMocks.callback = null + signalRMocks.onMoveJobUpdate.mockImplementation((callback: (job: MoveJobUpdate) => void) => { + signalRMocks.callback = callback + return signalRMocks.unsubscribe + }) }) - it('Change without moving should update audiobook and not call move API', async () => { + it('Change without moving plus metadata should call move API before metadata update', async () => { const wrapper = mount(EditAudiobookModal, { props: { isOpen: true, audiobook }, attachTo: document.body, @@ -67,10 +94,10 @@ describe('EditAudiobookModal move options', () => { // let init settle await new Promise((r) => setTimeout(r, 200)) - // Ensure there is a detectable change: set an explicit custom root and flip monitored + // Ensure there is a detectable change: set an explicit custom root and change title ;(wrapper.vm as unknown).selectedRootId = 0 ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\New Author\\New Book' - ;(wrapper.vm as unknown).formData.monitored = false + ;(wrapper.vm as unknown).formData.title = 'Sample Updated' await wrapper.vm.$nextTick() // Start save flow and resolve the in-component confirmation promise by @@ -85,8 +112,52 @@ describe('EditAudiobookModal move options', () => { await new Promise((r) => setTimeout(r, 50)) const { apiService } = await import('@/services/api') + expect(apiService.moveAudiobook).toHaveBeenCalledTimes(1) + expect(apiService.moveAudiobook).toHaveBeenCalledWith(1, 'C:/root/New Author/New Book', { + sourcePath: 'C:\\root\\Some Author\\Some Title', + moveFiles: false, + deleteEmptySource: false, + }) expect(apiService.updateAudiobook).toHaveBeenCalledTimes(1) - expect(apiService.moveAudiobook).toHaveBeenCalledTimes(0) + const updatePayload = vi.mocked(apiService.updateAudiobook).mock.calls[0][1] as Record< + string, + unknown + > + expect(updatePayload.title).toBe('Sample Updated') + expect(Object.prototype.hasOwnProperty.call(updatePayload, 'basePath')).toBe(false) + expect(Object.prototype.hasOwnProperty.call(updatePayload, 'imageUrl')).toBe(false) + expect(vi.mocked(apiService.moveAudiobook).mock.invocationCallOrder[0]).toBeLessThan( + vi.mocked(apiService.updateAudiobook).mock.invocationCallOrder[0], + ) + }) + + it('Destination-only change without moving should call move API and skip metadata update', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\New Author\\New Book' + await wrapper.vm.$nextTick() + + const savePromise = (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 10)) + const resolver = (wrapper.vm as unknown).moveConfirmResolver + if (resolver) resolver({ proceed: true, moveFiles: false, deleteEmptySource: true }) + await savePromise + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + expect(apiService.updateAudiobook).toHaveBeenCalledTimes(0) + expect(apiService.moveAudiobook).toHaveBeenCalledTimes(1) + expect(apiService.moveAudiobook).toHaveBeenCalledWith(1, 'C:/root/New Author/New Book', { + sourcePath: 'C:\\root\\Some Author\\Some Title', + moveFiles: false, + deleteEmptySource: false, + }) }) it('Move should call move API with deleteEmptySource true by default', async () => { @@ -117,14 +188,182 @@ describe('EditAudiobookModal move options', () => { const { apiService } = await import('@/services/api') expect(apiService.updateAudiobook).toHaveBeenCalledTimes(1) + expect(apiService.updateAudiobook).toHaveBeenCalledWith( + 1, + expect.not.objectContaining({ basePath: expect.anything() }), + ) expect(apiService.moveAudiobook).toHaveBeenCalledTimes(1) + expect(apiService.moveAudiobook).toHaveBeenCalledWith(1, 'C:/root/New Author/New Book', { + sourcePath: 'C:\\root\\Some Author\\Some Title', + moveFiles: true, + deleteEmptySource: true, + }) + }) + + it('Destination with parent traversal should be invalid and not call save APIs', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\Some Author\\Some Title\\..' + await wrapper.vm.$nextTick() + + expect(wrapper.text()).toContain('Path traversal is not allowed in the destination folder') + expect( + wrapper.find('button[aria-label="Save destination"]').attributes('disabled'), + ).toBeDefined() + + await (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + expect(apiService.updateAudiobook).toHaveBeenCalledTimes(0) + expect(apiService.moveAudiobook).toHaveBeenCalledTimes(0) + }) + + it('Destination segment with trailing whitespace should be invalid and not call save APIs', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\Some Author\\Some Title\\test ' + await wrapper.vm.$nextTick() + + expect(wrapper.text()).toContain( + 'Windows destination folder segments cannot end with a space or period', + ) + expect( + wrapper.find('button[aria-label="Save destination"]').attributes('disabled'), + ).toBeDefined() + + await (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + expect(apiService.updateAudiobook).toHaveBeenCalledTimes(0) + expect(apiService.moveAudiobook).toHaveBeenCalledTimes(0) + }) + + it('Destination inside current source should be allowed as a content move', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\Some Author\\Some Title\\ test' + await wrapper.vm.$nextTick() + + expect(wrapper.text()).not.toContain('Source and destination folders cannot overlap') + expect( + wrapper.find('button[aria-label="Save destination"]').attributes('disabled'), + ).toBeUndefined() + + const savePromise = (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 10)) + const resolver = (wrapper.vm as unknown).moveConfirmResolver + if (resolver) resolver({ proceed: true, moveFiles: true, deleteEmptySource: true }) + await savePromise + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + expect(apiService.updateAudiobook).toHaveBeenCalledTimes(0) expect(apiService.moveAudiobook).toHaveBeenCalledWith( - expect.anything(), - expect.anything(), - expect.objectContaining({ moveFiles: true, deleteEmptySource: true }), + 1, + 'C:/root/Some Author/Some Title/ test', + { + sourcePath: 'C:\\root\\Some Author\\Some Title', + moveFiles: true, + deleteEmptySource: true, + }, ) }) + it('Windows destination segment with leading whitespace outside source should be allowed', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\Some Author\\Other Title\\ test' + await wrapper.vm.$nextTick() + + expect(wrapper.text()).not.toContain('Windows destination folder segments cannot end') + expect( + wrapper.find('button[aria-label="Save destination"]').attributes('disabled'), + ).toBeUndefined() + + const savePromise = (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 10)) + const resolver = (wrapper.vm as unknown).moveConfirmResolver + if (resolver) resolver({ proceed: true, moveFiles: true, deleteEmptySource: true }) + await savePromise + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + expect(apiService.moveAudiobook).toHaveBeenCalledWith( + 1, + 'C:/root/Some Author/Other Title/ test', + { + sourcePath: 'C:\\root\\Some Author\\Some Title', + moveFiles: true, + deleteEmptySource: true, + }, + ) + }) + + it('Move-only destination changes should enqueue move without pre-saving BasePath', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\New Author\\New Book' + await wrapper.vm.$nextTick() + + const savePromise = (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 10)) + const resolver = (wrapper.vm as unknown).moveConfirmResolver + if (resolver) resolver({ proceed: true, moveFiles: true, deleteEmptySource: true }) + await savePromise + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + const { useMoveJobsStore } = await import('@/stores/moveJobs') + const moveJobsStore = useMoveJobsStore() + expect(apiService.updateAudiobook).toHaveBeenCalledTimes(0) + expect(apiService.moveAudiobook).toHaveBeenCalledWith(1, 'C:/root/New Author/New Book', { + sourcePath: 'C:\\root\\Some Author\\Some Title', + moveFiles: true, + deleteEmptySource: true, + }) + expect(moveJobsStore.trackedById['job-1']).toEqual({ + jobId: 'job-1', + audiobookId: 1, + status: 'Queued', + target: 'C:/root/New Author/New Book', + }) + expect(signalRMocks.onMoveJobUpdate).toHaveBeenCalledTimes(1) + expect(wrapper.emitted('saved')).toHaveLength(1) + expect(wrapper.emitted('close')).toHaveLength(1) + }) + it('Edition-only changes should persist through updateAudiobook', async () => { const wrapper = mount(EditAudiobookModal, { props: { isOpen: true, audiobook }, @@ -147,6 +386,44 @@ describe('EditAudiobookModal move options', () => { ) }) + it('metadata edit with separator-only custom Windows path does not enqueue a move', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + + const vm = wrapper.vm as unknown as { + selectedRootId: number + customRootPath: string + formData: { title: string } + handleSave: () => Promise + } + vm.selectedRootId = 0 + vm.customRootPath = 'C:/root/Some Author/Some Title' + vm.formData.title = 'Updated Sample' + await wrapper.vm.$nextTick() + + expect(wrapper.text()).not.toContain('Destination folder must be different') + + await vm.handleSave() + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + expect(apiService.updateAudiobook).toHaveBeenCalledTimes(1) + expect(apiService.updateAudiobook).toHaveBeenCalledWith( + 1, + expect.objectContaining({ title: 'Updated Sample' }), + ) + expect(apiService.updateAudiobook).toHaveBeenCalledWith( + 1, + expect.not.objectContaining({ basePath: expect.anything() }), + ) + expect(apiService.moveAudiobook).toHaveBeenCalledTimes(0) + }) + it('metadata changes should persist through updateAudiobook', async () => { const wrapper = mount(EditAudiobookModal, { props: { diff --git a/fe/src/__tests__/RootFolderFormModal.spec.ts b/fe/src/__tests__/RootFolderFormModal.spec.ts new file mode 100644 index 000000000..976df40f6 --- /dev/null +++ b/fe/src/__tests__/RootFolderFormModal.spec.ts @@ -0,0 +1,54 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { mount } from '@vue/test-utils' +import { createPinia, setActivePinia } from 'pinia' +import RootFolderFormModal from '@/components/settings/RootFolderFormModal.vue' +import { useRootFoldersStore } from '@/stores/rootFolders' + +const success = vi.fn() +const error = vi.fn() + +vi.mock('@/services/toastService', () => ({ + useToast: () => ({ success, error }), +})) + +describe('RootFolderFormModal', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it.each([ + [true, 'Root relocation started'], + [false, 'Root path metadata updated'], + ])('reports the path change accurately when moveFiles is %s', async (moveFiles, message) => { + const pinia = createPinia() + setActivePinia(pinia) + const store = useRootFoldersStore() + vi.spyOn(store, 'update').mockResolvedValue({ + id: 7, + name: 'Library', + path: '/new-library', + isDefault: true, + }) + const wrapper = mount(RootFolderFormModal, { + props: { + root: { + id: 7, + name: 'Library', + path: '/old-library', + isDefault: true, + }, + }, + global: { + plugins: [pinia], + stubs: { + FolderBrowserModal: true, + }, + }, + }) + await wrapper.get('#root-path').setValue('/new-library') + await ( + wrapper.vm as unknown as { confirmChange: (moveFiles: boolean) => Promise } + ).confirmChange(moveFiles) + await vi.waitFor(() => expect(success).toHaveBeenCalledWith('Success', message)) + }) +}) diff --git a/fe/src/__tests__/debug_AddNew.spec.ts b/fe/src/__tests__/debug_AddNew.spec.ts deleted file mode 100644 index 932f46770..000000000 --- a/fe/src/__tests__/debug_AddNew.spec.ts +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Listenarr - Audiobook Management System - * Copyright (C) 2024-2026 Listenarr Contributors - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published - * by the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ -import { describe } from 'vitest' - -describe.todo('debug AddNew placeholder') diff --git a/fe/src/__tests__/moveJobs.store.spec.ts b/fe/src/__tests__/moveJobs.store.spec.ts new file mode 100644 index 000000000..6cd8035ee --- /dev/null +++ b/fe/src/__tests__/moveJobs.store.spec.ts @@ -0,0 +1,237 @@ +/* + * Listenarr - Audiobook Management System + * Copyright (C) 2024-2026 Listenarr Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createPinia, setActivePinia } from 'pinia' + +type MoveJobUpdate = { + jobId?: string + audiobookId?: number + status?: string + target?: string + error?: string +} + +const toastMocks = vi.hoisted(() => ({ + info: vi.fn(), + success: vi.fn(), + error: vi.fn(), +})) + +const apiMocks = vi.hoisted(() => ({ + getMoveJobStatus: vi.fn(), +})) + +const signalRMocks = vi.hoisted(() => { + const state = { + callback: null as ((job: MoveJobUpdate) => void) | null, + unsubscribe: vi.fn(), + onMoveJobUpdate: vi.fn(), + } + state.onMoveJobUpdate.mockImplementation((callback: (job: MoveJobUpdate) => void) => { + state.callback = callback + return state.unsubscribe + }) + return state +}) + +vi.mock('@/services/api', () => ({ + apiService: { + getMoveJobStatus: apiMocks.getMoveJobStatus, + }, +})) + +vi.mock('@/services/toastService', () => ({ + useToast: () => toastMocks, +})) + +vi.mock('@/services/signalr', () => ({ + signalRService: { + onMoveJobUpdate: signalRMocks.onMoveJobUpdate, + }, +})) + +import { useMoveJobsStore } from '@/stores/moveJobs' + +describe('move jobs store', () => { + beforeEach(() => { + vi.clearAllMocks() + setActivePinia(createPinia()) + signalRMocks.callback = null + apiMocks.getMoveJobStatus.mockImplementation(() => new Promise(() => {})) + signalRMocks.onMoveJobUpdate.mockImplementation((callback: (job: MoveJobUpdate) => void) => { + signalRMocks.callback = callback + return signalRMocks.unsubscribe + }) + }) + + it('starts SignalR subscription idempotently', () => { + const store = useMoveJobsStore() + + store.start() + store.start() + + expect(signalRMocks.onMoveJobUpdate).toHaveBeenCalledTimes(1) + + store.stop() + expect(signalRMocks.unsubscribe).toHaveBeenCalledTimes(1) + }) + + it('tracks queued move jobs and subscribes on first track', () => { + const store = useMoveJobsStore() + + store.trackQueuedJob({ + jobId: 'JOB-1', + audiobookId: 42, + target: '/library/book', + }) + + expect(signalRMocks.onMoveJobUpdate).toHaveBeenCalledTimes(1) + expect(store.trackedById['job-1']).toEqual({ + jobId: 'JOB-1', + audiobookId: 42, + status: 'Queued', + target: '/library/book', + }) + }) + + it('shows one in-progress toast when a tracked job starts running', () => { + const store = useMoveJobsStore() + store.trackQueuedJob({ jobId: 'job-1', target: '/library/book' }) + + signalRMocks.callback?.({ jobId: 'job-1', status: 'Running', target: '/library/book' }) + signalRMocks.callback?.({ jobId: 'job-1', status: 'Running', target: '/library/book' }) + + expect(toastMocks.info).toHaveBeenCalledTimes(1) + expect(toastMocks.info).toHaveBeenCalledWith( + 'Move in progress', + 'Moving files to /library/book', + ) + expect(store.trackedById['job-1']?.status).toBe('Running') + }) + + it('shows success toast and clears tracked job on completion', () => { + const store = useMoveJobsStore() + store.trackQueuedJob({ jobId: 'job-1', target: '/library/book' }) + + signalRMocks.callback?.({ jobId: 'job-1', status: 'Completed', target: '/library/book' }) + + expect(toastMocks.success).toHaveBeenCalledWith( + 'Move completed', + 'Files moved to /library/book', + ) + expect(store.trackedById['job-1']).toBeUndefined() + }) + + it('shows attention toast and clears tracked job on NeedsAttention', () => { + const store = useMoveJobsStore() + store.trackQueuedJob({ jobId: 'job-1', target: '/library/book' }) + + signalRMocks.callback?.({ + jobId: 'job-1', + status: 'NeedsAttention', + target: '/library/book', + error: 'Manual review required', + }) + + expect(toastMocks.error).toHaveBeenCalledWith('Move needs attention', 'Manual review required') + expect(store.trackedById['job-1']).toBeUndefined() + }) + + it('reconciles a job that completed before tracking began', async () => { + apiMocks.getMoveJobStatus.mockResolvedValue({ + jobId: 'job-1', + status: 'Completed', + target: '/library/book', + }) + const store = useMoveJobsStore() + + store.trackQueuedJob({ jobId: 'job-1', target: '/library/book' }) + + await vi.waitFor(() => expect(store.trackedById['job-1']).toBeUndefined()) + expect(toastMocks.success).toHaveBeenCalledWith( + 'Move completed', + 'Files moved to /library/book', + ) + }) + + it('does not recreate a terminal job when a stale status response arrives later', async () => { + let resolveStatus: + | ((value: { jobId: string; status: string; target: string }) => void) + | undefined + apiMocks.getMoveJobStatus.mockImplementation( + () => + new Promise((resolve) => { + resolveStatus = resolve + }), + ) + const store = useMoveJobsStore() + store.trackQueuedJob({ jobId: 'job-1', target: '/library/book' }) + + signalRMocks.callback?.({ jobId: 'job-1', status: 'Completed', target: '/library/book' }) + resolveStatus?.({ jobId: 'job-1', status: 'Queued', target: '/library/book' }) + await Promise.resolve() + + expect(store.trackedById['job-1']).toBeUndefined() + expect(toastMocks.success).toHaveBeenCalledTimes(1) + }) + + it('does not regress a running job when a stale queued status response arrives', async () => { + let resolveStatus: + | ((value: { jobId: string; status: string; target: string }) => void) + | undefined + apiMocks.getMoveJobStatus.mockImplementation( + () => + new Promise((resolve) => { + resolveStatus = resolve + }), + ) + const store = useMoveJobsStore() + store.trackQueuedJob({ jobId: 'job-1', target: '/library/book' }) + + signalRMocks.callback?.({ jobId: 'job-1', status: 'Running', target: '/library/book' }) + resolveStatus?.({ jobId: 'job-1', status: 'Queued', target: '/library/book' }) + await Promise.resolve() + + expect(store.trackedById['job-1']?.status).toBe('Running') + expect(toastMocks.info).toHaveBeenCalledTimes(1) + }) + + it('keeps tracking when status reconciliation fails', async () => { + apiMocks.getMoveJobStatus.mockRejectedValue(new Error('offline')) + const store = useMoveJobsStore() + + store.trackQueuedJob({ jobId: 'job-1', target: '/library/book' }) + + await vi.waitFor(() => expect(apiMocks.getMoveJobStatus).toHaveBeenCalledWith('job-1')) + expect(store.trackedById['job-1']?.status).toBe('Queued') + expect(toastMocks.error).not.toHaveBeenCalled() + }) + + it('shows terminal error toast and clears tracked job on Superseded', () => { + const store = useMoveJobsStore() + store.trackQueuedJob({ jobId: 'job-1', target: '/library/book' }) + + signalRMocks.callback?.({ jobId: 'job-1', status: 'Superseded', target: '/library/book' }) + + expect(toastMocks.error).toHaveBeenCalledWith( + 'Move failed', + 'Move job did not complete. Check the move queue.', + ) + expect(store.trackedById['job-1']).toBeUndefined() + }) +}) diff --git a/fe/src/__tests__/utils/path.spec.ts b/fe/src/__tests__/utils/path.spec.ts index 88ece48b9..107cb9849 100644 --- a/fe/src/__tests__/utils/path.spec.ts +++ b/fe/src/__tests__/utils/path.spec.ts @@ -21,7 +21,21 @@ import { trimTrailingSlash, normalizeForCompare, isAbsolutePath, + hasRelativePathSegment, + hasParentTraversalSegment, + hasEmptyMiddlePathSegment, + hasControlCharacter, + hasOuterWhitespace, + hasPathSegmentOuterWhitespace, + hasWindowsTrailingSpaceOrPeriodSegment, + hasWindowsInvalidCharacter, + pathsOverlap, + pathsEqual, + pathIsInside, + hasWindowsReservedDeviceSegment, + validateLibraryDestinationPath, stripRootPrefix, + detectPathKind, } from '@/utils/path' describe('path utils', () => { @@ -34,6 +48,8 @@ describe('path utils', () => { expect(trimTrailingSlash('C:/path/')).toBe('C:/path') expect(trimTrailingSlash('C:\\path\\')).toBe('C:\\path') expect(trimTrailingSlash('no-slash')).toBe('no-slash') + expect(trimTrailingSlash('/')).toBe('/') + expect(trimTrailingSlash('C:\\')).toBe('C:\\') }) it('normalizeForCompare lowercases and trims', () => { @@ -46,6 +62,134 @@ describe('path utils', () => { expect(isAbsolutePath('relative/path')).toBe(false) }) + it('classifies absolute Unix paths with backslashes as Unix paths', () => { + expect(detectPathKind('/books/Author\\Name')).toBe('unix') + expect(normalizeForCompare('/books/Author\\Name')).toBe('/books/Author\\Name') + }) + + it('detects exact relative path segments without blocking periods in names', () => { + expect(hasRelativePathSegment('D:\\Books\\Title\\.')).toBe(true) + expect(hasRelativePathSegment('D:\\Books\\Title\\..')).toBe(true) + expect(hasRelativePathSegment('/books/./title')).toBe(true) + expect(hasRelativePathSegment('/books/../title')).toBe(true) + expect(hasRelativePathSegment('/books/Dr. Seuss')).toBe(false) + expect(hasRelativePathSegment('/books/.metadata')).toBe(false) + expect(hasRelativePathSegment('/books/title...')).toBe(false) + }) + + it('hasParentTraversalSegment detects parent directory traversal', () => { + expect(hasParentTraversalSegment('D:\\Books\\Title\\..')).toBe(true) + expect(hasParentTraversalSegment('/books/title/../other')).toBe(true) + expect(hasParentTraversalSegment('/books/title..')).toBe(false) + expect(hasParentTraversalSegment('/books/.../title')).toBe(false) + expect(hasParentTraversalSegment(null)).toBe(false) + }) + + it('detects empty middle path segments without rejecting roots', () => { + expect(hasEmptyMiddlePathSegment('D:\\Books\\\\Title')).toBe(true) + expect(hasEmptyMiddlePathSegment('/books//title')).toBe(true) + expect(hasEmptyMiddlePathSegment('D:\\Books\\Title')).toBe(false) + expect(hasEmptyMiddlePathSegment('/books/title')).toBe(false) + expect(hasEmptyMiddlePathSegment('D:\\')).toBe(false) + expect(hasEmptyMiddlePathSegment('\\\\server\\share\\Audiobooks')).toBe(false) + expect(hasEmptyMiddlePathSegment('\\\\server\\share\\\\Audiobooks')).toBe(true) + }) + + it('detects control characters and segment whitespace', () => { + expect(hasControlCharacter('D:\\Books\\Title\n')).toBe(true) + expect(hasControlCharacter('D:\\Books\\Title')).toBe(false) + expect(hasOuterWhitespace(' D:\\Books\\Title')).toBe(true) + expect(hasOuterWhitespace('D:\\Books\\Title ')).toBe(true) + expect(hasOuterWhitespace('D:\\Listenarr Test\\Title')).toBe(false) + expect(hasPathSegmentOuterWhitespace('D:\\Books\\test ')).toBe(true) + expect(hasPathSegmentOuterWhitespace('D:\\Books\\ test')).toBe(true) + expect(hasPathSegmentOuterWhitespace('D:\\Listenarr Test\\Title')).toBe(false) + }) + + it('detects Windows-only trailing space or period segments', () => { + expect(hasWindowsTrailingSpaceOrPeriodSegment('D:\\Books\\test ')).toBe(true) + expect(hasWindowsTrailingSpaceOrPeriodSegment('D:\\Books\\test.')).toBe(true) + expect(hasWindowsTrailingSpaceOrPeriodSegment('D:\\Books\\ test')).toBe(false) + expect(hasWindowsTrailingSpaceOrPeriodSegment('/books/test ')).toBe(false) + expect(hasWindowsTrailingSpaceOrPeriodSegment('/books/ test ')).toBe(false) + }) + + it('detects Windows invalid characters and reserved device names', () => { + expect(hasWindowsInvalidCharacter('D:\\Books\\Bad|Folder')).toBe(true) + expect(hasWindowsInvalidCharacter('D:\\Books\\Bad:Folder')).toBe(true) + expect(hasWindowsInvalidCharacter('D:\\Books\\Good Folder')).toBe(false) + expect(hasWindowsReservedDeviceSegment('D:\\Books\\CON')).toBe(true) + expect(hasWindowsReservedDeviceSegment('D:\\Books\\NUL.txt')).toBe(true) + expect(hasWindowsReservedDeviceSegment('D:\\Books\\COM1.folder')).toBe(true) + expect(hasWindowsReservedDeviceSegment('D:\\Books\\Concert')).toBe(false) + }) + + it('detects overlapping source and destination paths', () => { + expect(pathsOverlap('D:\\Books\\Title\\Child', 'D:\\Books\\Title', 'windows')).toBe(true) + expect(pathsOverlap('D:\\Books\\Title', 'D:\\Books\\Title\\Child', 'windows')).toBe(true) + expect(pathsOverlap('D:\\Books\\Title2', 'D:\\Books\\Title', 'windows')).toBe(false) + expect(pathsOverlap('/books/title/child', '/books/title', 'unix')).toBe(true) + expect(pathsOverlap('/books/title2', '/books/title', 'unix')).toBe(false) + expect(pathsOverlap('/Books/title', '/books', 'unix')).toBe(false) + expect(pathIsInside('/Author/Title', '/', 'unix')).toBe(true) + }) + + it('uses server-provided case sensitivity instead of path shape', () => { + expect(pathsEqual('/Books/Title', '/books/title', 'unix', 'Insensitive')).toBe(true) + expect(pathsEqual('C:\\Books\\Title', 'c:\\books\\title', 'windows', 'Sensitive')).toBe(false) + expect(pathIsInside('/Books/Title', '/books', 'unix', 'Insensitive')).toBe(true) + }) + + it('validates library destination paths while allowing platform-valid whitespace', () => { + expect(validateLibraryDestinationPath('D:\\Books\\Title\\.')).toContain( + 'current-directory path segments', + ) + expect(validateLibraryDestinationPath('/books/title/.')).toContain( + 'current-directory path segments', + ) + expect(validateLibraryDestinationPath('D:\\Books\\Title\\..')).toContain( + 'Path traversal is not allowed', + ) + expect(validateLibraryDestinationPath('/books/title/..')).toContain( + 'Path traversal is not allowed', + ) + expect(validateLibraryDestinationPath('D:\\Books\\\\Title')).toContain('empty path segments') + expect(validateLibraryDestinationPath('D:\\Books\\Bad*Folder')).toContain('invalid on Windows') + expect(validateLibraryDestinationPath('D:\\Books\\CON.txt')).toContain('reserved Windows') + expect(validateLibraryDestinationPath('D:\\Books\\test ')).toContain( + 'cannot end with a space or period', + ) + expect(validateLibraryDestinationPath('D:\\Books\\test.')).toContain( + 'cannot end with a space or period', + ) + expect(validateLibraryDestinationPath('D:\\Books\\ test')).toBe(null) + expect(validateLibraryDestinationPath('/books/ test /')).toBe(null) + expect(validateLibraryDestinationPath('D:\\Books\\Dr. Seuss')).toBe(null) + expect(validateLibraryDestinationPath('D:\\Books\\.metadata')).toBe(null) + expect(validateLibraryDestinationPath('D:\\Books\\Title...')).toContain( + 'cannot end with a space or period', + ) + expect(validateLibraryDestinationPath('/books/Title...')).toBe(null) + expect( + validateLibraryDestinationPath('D:\\Books\\Title\\Child', { + pathKind: 'windows', + sourcePath: 'D:\\Books\\Title', + }), + ).toBe(null) + expect( + validateLibraryDestinationPath('/books/title/child', { + pathKind: 'unix', + sourcePath: '/books/title', + }), + ).toBe(null) + expect( + validateLibraryDestinationPath('D:\\Books', { + pathKind: 'windows', + sourcePath: 'D:\\Books\\Title', + }), + ).toBe(null) + }) + it('stripRootPrefix removes root prefix when present', () => { const root = 'C:\\temp\\Isaac Asimov\\Foundation' const full = 'C:\\temp\\Isaac Asimov\\Foundation\\Prelude to Foundation' @@ -60,6 +204,8 @@ describe('path utils', () => { // returns null when no match expect(stripRootPrefix('C:/root/other', full)).toBe(null) + expect(stripRootPrefix('C:/root/books', 'C:/root/bookshelf/Title')).toBe(null) + expect(stripRootPrefix('C:/root/books/Extra', 'C:/other/root/bookshelf/Title')).toBe(null) // matches using last segments const root3 = 'C:/temp/Isaac Asimov/Foundation/Extra' diff --git a/fe/src/components/domain/audiobook/EditAudiobookModal.vue b/fe/src/components/domain/audiobook/EditAudiobookModal.vue index 7632ab9dd..e18fbfe21 100644 --- a/fe/src/components/domain/audiobook/EditAudiobookModal.vue +++ b/fe/src/components/domain/audiobook/EditAudiobookModal.vue @@ -504,8 +504,9 @@ type="button" class="btn icon-btn btn-primary btn-sm" @click="finishEditingDestination" + :disabled="Boolean(destinationPathValidationError)" aria-label="Save destination" - title="Done" + :title="destinationPathValidationError || 'Done'" > @@ -524,6 +525,10 @@ organizing within the selected root.

+
+ + {{ destinationPathValidationError }} +
@@ -723,20 +728,12 @@ > Close -
- - Move Job: {{ moveJob.jobId }} — {{ moveJob.status }} - -
- Target: {{ moveJob.target }} -
-