From 60ad384bd03e5ea4774ffba5f5cd699e4263a0a4 Mon Sep 17 00:00:00 2001 From: Brent Date: Thu, 17 Sep 2026 11:53:22 -0700 Subject: [PATCH 1/5] Allow conversion to be applied to books already in the library MP3 -> M4B conversion only ran for files arriving from a tracked download client, so a collection that predates Chaptarr could never be normalised to one format. ConvertBookGroupIfNeeded returned early whenever there was no DownloadClientItem, and nothing else could reach the converter. The download id was only ever a correlation key, so it is now synthesised from the book and edition when no download client owns the import. That alone lets a manual import convert. On top of it, a ConvertBookFiles command (and a bulk ConvertAuthor) hands existing BookFile rows back to the import pipeline as a library conversion, reusing ffmpeg detection, the workspace free-space check, the concurrency semaphore, tagging, chapters, destination naming and the recycle bin rather than duplicating any of it. A library conversion replaces exactly the files it converted. It does not inherit the manual-import behaviour of replacing every other file on the book, so converting an audiobook leaves the book's eBook and any second audiobook edition alone. It also bypasses quality gating, since the profile that wants everything as M4B is usually the one that disallows MP3. Selection is per book in the UI and expanded to the whole edition in the service: every convertible file of a book becomes one output file, so converting a subset would leave the book half MP3 and half M4B. - GET /api/v1/convert?authorId=&bookId= previews what would convert and why a file would be skipped - Convert Files toolbar action on the author and book detail pages - Detached conversion jobs stay limited to tracked downloads, which is the only path that has a way to resume the import Fixes #109 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GXPbRG8Geef4zMeMQgpQ8H --- frontend/src/Author/Details/AuthorDetails.js | 24 + frontend/src/Book/Details/BookDetails.js | 25 + frontend/src/Commands/commandNames.js | 2 + frontend/src/Convert/ConvertPreviewModal.js | 34 ++ .../Convert/ConvertPreviewModalConnector.js | 39 ++ .../Convert/ConvertPreviewModalContent.css | 37 ++ .../ConvertPreviewModalContent.css.d.ts | 13 + .../src/Convert/ConvertPreviewModalContent.js | 282 +++++++++++ .../ConvertPreviewModalContentConnector.js | 76 +++ frontend/src/Convert/ConvertPreviewRow.css | 31 ++ .../src/Convert/ConvertPreviewRow.css.d.ts | 11 + frontend/src/Convert/ConvertPreviewRow.js | 70 +++ frontend/src/Helpers/Props/icons.js | 2 + .../Store/Actions/convertPreviewActions.js | 51 ++ frontend/src/Store/Actions/index.js | 2 + .../Books/ConvertBookController.cs | 34 ++ .../Books/ConvertBookResource.cs | 53 ++ .../BookFileConversionServiceFixture.cs | 440 ++++++++++++++++ src/NzbDrone.Core/Localization/Core/en.json | 7 + .../MediaFiles/BookFileConversionService.cs | 477 ++++++++++++++++++ .../BookImport/ImportApprovedBooks.cs | 120 ++++- .../Commands/ConvertAuthorCommand.cs | 24 + .../Commands/ConvertBookFilesCommand.cs | 32 ++ src/NzbDrone.Core/Parser/Model/LocalBook.cs | 6 + 24 files changed, 1864 insertions(+), 28 deletions(-) create mode 100644 frontend/src/Convert/ConvertPreviewModal.js create mode 100644 frontend/src/Convert/ConvertPreviewModalConnector.js create mode 100644 frontend/src/Convert/ConvertPreviewModalContent.css create mode 100644 frontend/src/Convert/ConvertPreviewModalContent.css.d.ts create mode 100644 frontend/src/Convert/ConvertPreviewModalContent.js create mode 100644 frontend/src/Convert/ConvertPreviewModalContentConnector.js create mode 100644 frontend/src/Convert/ConvertPreviewRow.css create mode 100644 frontend/src/Convert/ConvertPreviewRow.css.d.ts create mode 100644 frontend/src/Convert/ConvertPreviewRow.js create mode 100644 frontend/src/Store/Actions/convertPreviewActions.js create mode 100644 src/Chaptarr.Api.V1/Books/ConvertBookController.cs create mode 100644 src/Chaptarr.Api.V1/Books/ConvertBookResource.cs create mode 100644 src/Chaptarr.Core.Test/MediaFiles/BookFileConversionServiceFixture.cs create mode 100644 src/NzbDrone.Core/MediaFiles/BookFileConversionService.cs create mode 100644 src/NzbDrone.Core/MediaFiles/Commands/ConvertAuthorCommand.cs create mode 100644 src/NzbDrone.Core/MediaFiles/Commands/ConvertBookFilesCommand.cs diff --git a/frontend/src/Author/Details/AuthorDetails.js b/frontend/src/Author/Details/AuthorDetails.js index cf4a5cf9..f183e473 100644 --- a/frontend/src/Author/Details/AuthorDetails.js +++ b/frontend/src/Author/Details/AuthorDetails.js @@ -18,6 +18,7 @@ import PageToolbarButton from 'Components/Page/Toolbar/PageToolbarButton'; import PageToolbarSection from 'Components/Page/Toolbar/PageToolbarSection'; import PageToolbarSeparator from 'Components/Page/Toolbar/PageToolbarSeparator'; import SwipeHeaderConnector from 'Components/Swipe/SwipeHeaderConnector'; +import ConvertPreviewModalConnector from 'Convert/ConvertPreviewModalConnector'; import { align, icons, kinds } from 'Helpers/Props'; import InteractiveSearchFilterMenuConnector from 'InteractiveSearch/InteractiveSearchFilterMenuConnector'; import InteractiveSearchTable from 'InteractiveSearch/InteractiveSearchTable'; @@ -55,6 +56,7 @@ class AuthorDetails extends Component { this.state = { isOrganizeModalOpen: false, isRetagModalOpen: false, + isConvertModalOpen: false, isEditAuthorModalOpen: false, isSelectRootFolderModalOpen: false, selectRootFolderMediaType: 'audiobook', @@ -130,6 +132,14 @@ class AuthorDetails extends Component { this.setState({ isRetagModalOpen: false }); }; + onConvertPress = () => { + this.setState({ isConvertModalOpen: true }); + }; + + onConvertModalClose = () => { + this.setState({ isConvertModalOpen: false }); + }; + onInteractiveImportPress = () => { this.setState({ isInteractiveImportModalOpen: true }); }; @@ -281,6 +291,7 @@ class AuthorDetails extends Component { const { isOrganizeModalOpen, isRetagModalOpen, + isConvertModalOpen, isEditAuthorModalOpen, isSelectRootFolderModalOpen, selectRootFolderMediaType, @@ -345,6 +356,13 @@ class AuthorDetails extends Component { onPress={this.onRetagPress} /> + + + + { + this.setState({ isConvertModalOpen: true }); + }; + + onConvertModalClose = () => { + this.setState({ isConvertModalOpen: false }); + }; + onEditBookPress = () => { this.setState({ isEditBookModalOpen: true }); }; @@ -129,6 +139,7 @@ class BookDetails extends Component { const { isOrganizeModalOpen, isRetagModalOpen, + isConvertModalOpen, isEditBookModalOpen, isDeleteBookModalOpen, selectedTabIndex @@ -170,6 +181,13 @@ class BookDetails extends Component { onPress={this.onRetagPress} /> + + + + + { + isOpen && + + } + + ); +} + +ConvertPreviewModal.propTypes = { + isOpen: PropTypes.bool.isRequired, + onModalClose: PropTypes.func.isRequired +}; + +export default ConvertPreviewModal; diff --git a/frontend/src/Convert/ConvertPreviewModalConnector.js b/frontend/src/Convert/ConvertPreviewModalConnector.js new file mode 100644 index 00000000..ce30d8a5 --- /dev/null +++ b/frontend/src/Convert/ConvertPreviewModalConnector.js @@ -0,0 +1,39 @@ +import PropTypes from 'prop-types'; +import React, { Component } from 'react'; +import { connect } from 'react-redux'; +import { clearConvertPreview } from 'Store/Actions/convertPreviewActions'; +import ConvertPreviewModal from './ConvertPreviewModal'; + +const mapDispatchToProps = { + clearConvertPreview +}; + +class ConvertPreviewModalConnector extends Component { + + // + // Listeners + + onModalClose = () => { + this.props.clearConvertPreview(); + this.props.onModalClose(); + }; + + // + // Render + + render() { + return ( + + ); + } +} + +ConvertPreviewModalConnector.propTypes = { + clearConvertPreview: PropTypes.func.isRequired, + onModalClose: PropTypes.func.isRequired +}; + +export default connect(undefined, mapDispatchToProps)(ConvertPreviewModalConnector); diff --git a/frontend/src/Convert/ConvertPreviewModalContent.css b/frontend/src/Convert/ConvertPreviewModalContent.css new file mode 100644 index 00000000..8c4e531e --- /dev/null +++ b/frontend/src/Convert/ConvertPreviewModalContent.css @@ -0,0 +1,37 @@ +.previews { + margin-top: 20px; +} + +.book { + margin-bottom: 20px; + + &:last-of-type { + margin-bottom: 0; + } +} + +.bookHeader { + display: flex; + align-items: center; + margin-bottom: 8px; +} + +.bookSelectedContainer { + margin-bottom: 0; + margin-right: 10px; +} + +.bookTitle { + font-weight: bold; +} + +.selectAllInputContainer { + margin-right: auto; + padding-top: 6px; + padding-left: 15px; + font-size: 20px; +} + +.selectAllInput { + margin-bottom: 0; +} diff --git a/frontend/src/Convert/ConvertPreviewModalContent.css.d.ts b/frontend/src/Convert/ConvertPreviewModalContent.css.d.ts new file mode 100644 index 00000000..2c462efa --- /dev/null +++ b/frontend/src/Convert/ConvertPreviewModalContent.css.d.ts @@ -0,0 +1,13 @@ +// This file is automatically generated. +// Please do not change this file! +interface CssExports { + 'book': string; + 'bookHeader': string; + 'bookSelectedContainer': string; + 'bookTitle': string; + 'previews': string; + 'selectAllInput': string; + 'selectAllInputContainer': string; +} +export const cssExports: CssExports; +export default cssExports; diff --git a/frontend/src/Convert/ConvertPreviewModalContent.js b/frontend/src/Convert/ConvertPreviewModalContent.js new file mode 100644 index 00000000..0c2bf8c4 --- /dev/null +++ b/frontend/src/Convert/ConvertPreviewModalContent.js @@ -0,0 +1,282 @@ +import PropTypes from 'prop-types'; +import React, { Component } from 'react'; +import Alert from 'Components/Alert'; +import CheckInput from 'Components/Form/CheckInput'; +import Button from 'Components/Link/Button'; +import LoadingIndicator from 'Components/Loading/LoadingIndicator'; +import ModalBody from 'Components/Modal/ModalBody'; +import ModalContent from 'Components/Modal/ModalContent'; +import ModalFooter from 'Components/Modal/ModalFooter'; +import ModalHeader from 'Components/Modal/ModalHeader'; +import { kinds } from 'Helpers/Props'; +import translate from 'Utilities/String/translate'; +import ConvertPreviewRow from './ConvertPreviewRow'; +import styles from './ConvertPreviewModalContent.css'; + +// Every convertible file of a book becomes one output file, so a book is the smallest thing +// that can be converted. Selection is per book; the rows underneath are there to show what +// each one is made of and why anything is being left behind. +function groupByBook(items) { + const books = []; + const byEdition = new Map(); + + items.forEach((item) => { + let book = byEdition.get(item.editionId); + + if (!book) { + book = { + editionId: item.editionId, + title: item.bookTitle, + items: [], + fileIds: [] + }; + + byEdition.set(item.editionId, book); + books.push(book); + } + + book.items.push(item); + + if (item.canConvert) { + book.fileIds.push(item.bookFileId); + } + }); + + return books; +} + +function getSelectAllValue(selectedCount, convertibleCount) { + if (selectedCount === 0) { + return false; + } + + if (convertibleCount > 0 && selectedCount === convertibleCount) { + return true; + } + + return null; +} + +class ConvertPreviewModalContent extends Component { + + // + // Lifecycle + + constructor(props, context) { + super(props, context); + + this.state = { + selectedEditions: {}, + hasInitialized: false + }; + } + + static getDerivedStateFromProps(props, state) { + if (state.hasInitialized || !props.isPopulated || props.isFetching) { + return null; + } + + // Default to converting everything that can be converted. + const selectedEditions = {}; + + groupByBook(props.items).forEach((book) => { + if (book.fileIds.length) { + selectedEditions[book.editionId] = true; + } + }); + + return { + selectedEditions, + hasInitialized: true + }; + } + + // + // Control + + getSelectedBooks = () => { + return groupByBook(this.props.items) + .filter((book) => book.fileIds.length && this.state.selectedEditions[book.editionId]); + }; + + // + // Listeners + + onSelectAllChange = ({ value }) => { + const selectedEditions = {}; + + groupByBook(this.props.items).forEach((book) => { + if (book.fileIds.length) { + selectedEditions[book.editionId] = value; + } + }); + + this.setState({ selectedEditions }); + }; + + onBookSelectedChange = ({ id, value }) => { + this.setState((state) => { + return { + selectedEditions: { + ...state.selectedEditions, + [id]: value + } + }; + }); + }; + + onConvertPress = () => { + const fileIds = this.getSelectedBooks().reduce((result, book) => { + return result.concat(book.fileIds); + }, []); + + this.props.onConvertPress(fileIds); + }; + + // + // Render + + render() { + const { + isFetching, + isPopulated, + error, + items, + onModalClose + } = this.props; + + const { selectedEditions } = this.state; + + const books = groupByBook(items); + const convertibleBooks = books.filter((book) => book.fileIds.length); + const selectedCount = this.getSelectedBooks().length; + const selectAllValue = getSelectAllValue(selectedCount, convertibleBooks.length); + + return ( + + + {translate('ConvertBookFiles')} + + + + { + isFetching && + + } + + { + !isFetching && error && +
+ {translate('ErrorLoadingPreviews')} +
+ } + + { + !isFetching && isPopulated && !items.length && +
+ {translate('NoFilesNeedConverting')} +
+ } + + { + !isFetching && isPopulated && !!items.length && +
+ + {translate('ConvertReplacesSourceFilesWarning')} + + + { + !convertibleBooks.length && + + {translate('ConvertNoEligibleFiles')} + + } + +
+ { + books.map((book) => { + const isConvertible = !!book.fileIds.length; + + return ( +
+
+ this.onBookSelectedChange({ id: book.editionId, value })} + /> + + + {book.title} + +
+ + { + book.items.map((item) => { + return ( + + ); + }) + } +
+ ); + }) + } +
+
+ } + + + + { + isPopulated && !!convertibleBooks.length && + + } + + + + + + + ); + } +} + +ConvertPreviewModalContent.propTypes = { + isFetching: PropTypes.bool.isRequired, + isPopulated: PropTypes.bool.isRequired, + error: PropTypes.object, + items: PropTypes.arrayOf(PropTypes.object).isRequired, + onConvertPress: PropTypes.func.isRequired, + onModalClose: PropTypes.func.isRequired +}; + +export default ConvertPreviewModalContent; diff --git a/frontend/src/Convert/ConvertPreviewModalContentConnector.js b/frontend/src/Convert/ConvertPreviewModalContentConnector.js new file mode 100644 index 00000000..85103479 --- /dev/null +++ b/frontend/src/Convert/ConvertPreviewModalContentConnector.js @@ -0,0 +1,76 @@ +import PropTypes from 'prop-types'; +import React, { Component } from 'react'; +import { connect } from 'react-redux'; +import { createSelector } from 'reselect'; +import * as commandNames from 'Commands/commandNames'; +import { executeCommand } from 'Store/Actions/commandActions'; +import { fetchConvertPreview } from 'Store/Actions/convertPreviewActions'; +import ConvertPreviewModalContent from './ConvertPreviewModalContent'; + +function createMapStateToProps() { + return createSelector( + (state) => state.convertPreview, + (convertPreview) => { + return { ...convertPreview }; + } + ); +} + +const mapDispatchToProps = { + fetchConvertPreview, + executeCommand +}; + +class ConvertPreviewModalContentConnector extends Component { + + // + // Lifecycle + + componentDidMount() { + const { + authorId, + bookId + } = this.props; + + this.props.fetchConvertPreview({ + authorId, + bookId + }); + } + + // + // Listeners + + onConvertPress = (files) => { + this.props.executeCommand({ + name: commandNames.CONVERT_BOOK_FILES, + authorId: this.props.authorId, + bookId: this.props.bookId, + files + }); + + this.props.onModalClose(); + }; + + // + // Render + + render() { + return ( + + ); + } +} + +ConvertPreviewModalContentConnector.propTypes = { + authorId: PropTypes.number.isRequired, + bookId: PropTypes.number, + fetchConvertPreview: PropTypes.func.isRequired, + executeCommand: PropTypes.func.isRequired, + onModalClose: PropTypes.func.isRequired +}; + +export default connect(createMapStateToProps, mapDispatchToProps)(ConvertPreviewModalContentConnector); diff --git a/frontend/src/Convert/ConvertPreviewRow.css b/frontend/src/Convert/ConvertPreviewRow.css new file mode 100644 index 00000000..b2eb66ac --- /dev/null +++ b/frontend/src/Convert/ConvertPreviewRow.css @@ -0,0 +1,31 @@ +.row { + display: flex; + margin-bottom: 5px; + padding: 5px 0 5px 34px; + border-bottom: 1px solid var(--borderColor); + + &:last-of-type { + margin-bottom: 0; + padding-bottom: 0; + border-bottom: none; + } +} + +.details { + overflow: hidden; +} + +.path { + margin-left: 10px; + word-break: break-all; +} + +.quality { + margin-left: 25px; + color: var(--textColor); +} + +.reason { + margin-top: 5px; + color: var(--warningColor); +} diff --git a/frontend/src/Convert/ConvertPreviewRow.css.d.ts b/frontend/src/Convert/ConvertPreviewRow.css.d.ts new file mode 100644 index 00000000..a321e5e6 --- /dev/null +++ b/frontend/src/Convert/ConvertPreviewRow.css.d.ts @@ -0,0 +1,11 @@ +// This file is automatically generated. +// Please do not change this file! +interface CssExports { + 'details': string; + 'path': string; + 'quality': string; + 'reason': string; + 'row': string; +} +export const cssExports: CssExports; +export default cssExports; diff --git a/frontend/src/Convert/ConvertPreviewRow.js b/frontend/src/Convert/ConvertPreviewRow.js new file mode 100644 index 00000000..3461ff58 --- /dev/null +++ b/frontend/src/Convert/ConvertPreviewRow.js @@ -0,0 +1,70 @@ +import PropTypes from 'prop-types'; +import React, { Component } from 'react'; +import Icon from 'Components/Icon'; +import { icons, kinds } from 'Helpers/Props'; +import translate from 'Utilities/String/translate'; +import styles from './ConvertPreviewRow.css'; + +class ConvertPreviewRow extends Component { + + // + // Render + + render() { + const { + path, + sourceQuality, + targetQuality, + canConvert, + reason + } = this.props; + + return ( +
+
+
+ + + + {path} + +
+ + { + canConvert && +
+ {translate('ConvertQualityChange', { sourceQuality, targetQuality })} +
+ } + + { + !canConvert && !!reason && +
+ + + + {reason} + +
+ } +
+
+ ); + } +} + +ConvertPreviewRow.propTypes = { + path: PropTypes.string.isRequired, + sourceQuality: PropTypes.string, + targetQuality: PropTypes.string, + canConvert: PropTypes.bool.isRequired, + reason: PropTypes.string +}; + +export default ConvertPreviewRow; diff --git a/frontend/src/Helpers/Props/icons.js b/frontend/src/Helpers/Props/icons.js index c1e0e973..203d7421 100644 --- a/frontend/src/Helpers/Props/icons.js +++ b/frontend/src/Helpers/Props/icons.js @@ -50,6 +50,7 @@ import { faDownload as fasDownload, faEdit as fasEdit, faEllipsisH as fasEllipsisH, + faExchangeAlt as fasExchangeAlt, faExclamationCircle as fasExclamationCircle, faExclamationTriangle as fasExclamationTriangle, faExternalLinkAlt as fasExternalLinkAlt, @@ -143,6 +144,7 @@ export const CLOSE = fasTimes; export const CLONE = farClone; export const COLLAPSE = fasChevronCircleUp; export const COMPUTER = fasDesktop; +export const CONVERT = fasExchangeAlt; export const DANGER = fasExclamationCircle; export const DELETE = fasTrashAlt; export const DOWNLOAD = fasDownload; diff --git a/frontend/src/Store/Actions/convertPreviewActions.js b/frontend/src/Store/Actions/convertPreviewActions.js new file mode 100644 index 00000000..a8ab8cab --- /dev/null +++ b/frontend/src/Store/Actions/convertPreviewActions.js @@ -0,0 +1,51 @@ +import { createAction } from 'redux-actions'; +import { createThunk, handleThunks } from 'Store/thunks'; +import createFetchHandler from './Creators/createFetchHandler'; +import createHandleActions from './Creators/createHandleActions'; + +// +// Variables + +export const section = 'convertPreview'; + +// +// State + +export const defaultState = { + isFetching: false, + isPopulated: false, + error: null, + items: [] +}; + +// +// Actions Types + +export const FETCH_CONVERT_PREVIEW = 'convertPreview/fetchConvertPreview'; +export const CLEAR_CONVERT_PREVIEW = 'convertPreview/clearConvertPreview'; + +// +// Action Creators + +export const fetchConvertPreview = createThunk(FETCH_CONVERT_PREVIEW); +export const clearConvertPreview = createAction(CLEAR_CONVERT_PREVIEW); + +// +// Action Handlers + +export const actionHandlers = handleThunks({ + + [FETCH_CONVERT_PREVIEW]: createFetchHandler('convertPreview', '/convert') + +}); + +// +// Reducers + +export const reducers = createHandleActions({ + + [CLEAR_CONVERT_PREVIEW]: (state) => { + return Object.assign({}, state, defaultState); + } + +}, defaultState, section); diff --git a/frontend/src/Store/Actions/index.js b/frontend/src/Store/Actions/index.js index 8c0047ca..b648b0d2 100644 --- a/frontend/src/Store/Actions/index.js +++ b/frontend/src/Store/Actions/index.js @@ -14,6 +14,7 @@ import * as bookStudio from './bookshelfActions'; import * as calendar from './calendarActions'; import * as captcha from './captchaActions'; import * as commands from './commandActions'; +import * as convertPreview from './convertPreviewActions'; import * as customFilters from './customFilterActions'; import * as editions from './editionActions'; import * as history from './historyActions'; @@ -53,6 +54,7 @@ export default [ calendar, captcha, commands, + convertPreview, customFilters, editions, history, diff --git a/src/Chaptarr.Api.V1/Books/ConvertBookController.cs b/src/Chaptarr.Api.V1/Books/ConvertBookController.cs new file mode 100644 index 00000000..e7bcc577 --- /dev/null +++ b/src/Chaptarr.Api.V1/Books/ConvertBookController.cs @@ -0,0 +1,34 @@ +using System.Collections.Generic; +using Chaptarr.Http; +using Microsoft.AspNetCore.Mvc; +using NzbDrone.Core.MediaFiles; + +namespace Chaptarr.Api.V1.Books +{ + [V1ApiController("convert")] + public class ConvertBookController : Controller + { + private readonly IBookFileConversionService _bookFileConversionService; + + public ConvertBookController(IBookFileConversionService bookFileConversionService) + { + _bookFileConversionService = bookFileConversionService; + } + + [HttpGet] + [ProducesResponseType(typeof(List), 200)] + [ProducesResponseType(typeof(ApiErrorResource), 400)] + public ActionResult> GetConvertibleBookFiles(int authorId, int? bookId) + { + if (authorId <= 0) + { + return BadRequest(new ApiErrorResource + { + Message = "authorId is required." + }); + } + + return _bookFileConversionService.GetConversionPreviews(authorId, bookId).ToResource(); + } + } +} diff --git a/src/Chaptarr.Api.V1/Books/ConvertBookResource.cs b/src/Chaptarr.Api.V1/Books/ConvertBookResource.cs new file mode 100644 index 00000000..f57e3c38 --- /dev/null +++ b/src/Chaptarr.Api.V1/Books/ConvertBookResource.cs @@ -0,0 +1,53 @@ +using System.Collections.Generic; +using System.Linq; +using Chaptarr.Http.REST; + +namespace Chaptarr.Api.V1.Books +{ + public class ConvertBookResource : RestResource + { + public int AuthorId { get; set; } + public int BookId { get; set; } + public int EditionId { get; set; } + public int BookFileId { get; set; } + public string BookTitle { get; set; } + public string Path { get; set; } + public long Size { get; set; } + public string SourceQuality { get; set; } + public string TargetQuality { get; set; } + public bool CanConvert { get; set; } + public string Reason { get; set; } + } + + public static class ConvertBookResourceMapper + { + public static ConvertBookResource ToResource(this NzbDrone.Core.MediaFiles.BookFileConversionPreview model) + { + if (model == null) + { + return null; + } + + return new ConvertBookResource + { + Id = model.BookFileId, + AuthorId = model.AuthorId, + BookId = model.BookId, + EditionId = model.EditionId, + BookFileId = model.BookFileId, + BookTitle = model.BookTitle, + Path = model.Path, + Size = model.Size, + SourceQuality = model.SourceQuality, + TargetQuality = model.TargetQuality, + CanConvert = model.CanConvert, + Reason = model.Reason + }; + } + + public static List ToResource(this IEnumerable models) + { + return models.Select(ToResource).ToList(); + } + } +} diff --git a/src/Chaptarr.Core.Test/MediaFiles/BookFileConversionServiceFixture.cs b/src/Chaptarr.Core.Test/MediaFiles/BookFileConversionServiceFixture.cs new file mode 100644 index 00000000..add712fc --- /dev/null +++ b/src/Chaptarr.Core.Test/MediaFiles/BookFileConversionServiceFixture.cs @@ -0,0 +1,440 @@ +using System; +using System.Collections.Generic; +using System.IO.Abstractions; +using System.Linq; +using System.Reflection; +using System.Threading; +using NLog; +using NUnit.Framework; +using NzbDrone.Common.Disk; +using NzbDrone.Core.Books; +using NzbDrone.Core.DecisionEngine; +using NzbDrone.Core.MediaFiles; +using NzbDrone.Core.MediaFiles.BookImport; +using NzbDrone.Core.MediaFiles.BookImport.Identification; +using NzbDrone.Core.MediaFiles.Commands; +using NzbDrone.Core.Parser.Model; +using NzbDrone.Core.Profiles.Qualities; +using NzbDrone.Core.Qualities; + +namespace Chaptarr.Core.Test.MediaFiles +{ + [TestFixture] + public class BookFileConversionServiceFixture + { + private const int AuthorId = 7; + private const int BookId = 42; + private const int EditionId = 84; + private const int OtherEditionId = 85; + + private IAuthorService _authorService; + private AuthorServiceProxy _authorServiceProxy; + private IMediaFileService _mediaFileService; + private MediaFileServiceProxy _mediaFileServiceProxy; + private StubImportApprovedBooks _importApprovedBooks; + private IDiskProvider _diskProvider; + private DiskProviderProxy _diskProviderProxy; + private BookFileConversionService _service; + + [SetUp] + public void Setup() + { + _authorService = DispatchProxy.Create(); + _authorServiceProxy = (AuthorServiceProxy)(object)_authorService; + _authorServiceProxy.Author = Author(convertToQualityId: Quality.M4B.Id); + + _mediaFileService = DispatchProxy.Create(); + _mediaFileServiceProxy = (MediaFileServiceProxy)(object)_mediaFileService; + + _diskProvider = DispatchProxy.Create(); + _diskProviderProxy = (DiskProviderProxy)(object)_diskProvider; + + _importApprovedBooks = new StubImportApprovedBooks(); + + _service = new BookFileConversionService( + _authorService, + DispatchProxy.Create(), + DispatchProxy.Create(), + _mediaFileService, + new StubImportDecisionMaker(), + _importApprovedBooks, + _diskProvider, + LogManager.GetCurrentClassLogger()); + } + + [Test] + public void should_list_mp3_files_as_convertible_when_profile_targets_m4b() + { + GivenFiles(Mp3File(1, @"C:\books\author\book\01.mp3")); + + var previews = _service.GetConversionPreviews(AuthorId); + + Assert.That(previews.Count, Is.EqualTo(1)); + Assert.That(previews[0].CanConvert, Is.True); + Assert.That(previews[0].SourceQuality, Is.EqualTo(Quality.MP3.Name)); + Assert.That(previews[0].TargetQuality, Is.EqualTo(Quality.M4B.Name)); + Assert.That(previews[0].BookId, Is.EqualTo(BookId)); + } + + [Test] + public void should_not_offer_to_convert_a_file_that_is_already_the_target_quality() + { + GivenFiles(File(2, @"C:\books\author\book\book.m4b", Quality.M4B)); + + var previews = _service.GetConversionPreviews(AuthorId); + + Assert.That(previews.Count, Is.EqualTo(1)); + Assert.That(previews[0].CanConvert, Is.False); + Assert.That(previews[0].Reason, Does.Contain("Already")); + } + + [Test] + public void should_explain_when_the_profile_has_no_conversion_target() + { + _authorServiceProxy.Author = Author(convertToQualityId: null); + GivenFiles(Mp3File(1, @"C:\books\author\book\01.mp3")); + + var previews = _service.GetConversionPreviews(AuthorId); + + Assert.That(previews.Count, Is.EqualTo(1)); + Assert.That(previews[0].CanConvert, Is.False); + Assert.That(previews[0].Reason, Does.Contain("no conversion target")); + } + + [Test] + public void should_explain_when_the_conversion_target_is_not_allowed_by_the_profile() + { + _authorServiceProxy.Author = Author(convertToQualityId: Quality.M4B.Id, allowM4b: false); + GivenFiles(Mp3File(1, @"C:\books\author\book\01.mp3")); + + var previews = _service.GetConversionPreviews(AuthorId); + + Assert.That(previews.Count, Is.EqualTo(1)); + Assert.That(previews[0].CanConvert, Is.False); + Assert.That(previews[0].Reason, Does.Contain("not allowed")); + } + + [Test] + public void should_not_offer_to_convert_a_file_that_is_missing_from_disk() + { + var file = Mp3File(1, @"C:\books\author\book\01.mp3"); + GivenFiles(file); + _diskProviderProxy.ExistingFiles.Remove(file.Path); + + var previews = _service.GetConversionPreviews(AuthorId); + + Assert.That(previews.Count, Is.EqualTo(1)); + Assert.That(previews[0].CanConvert, Is.False); + Assert.That(previews[0].Reason, Does.Contain("missing")); + } + + [Test] + public void should_leave_ebooks_out_of_the_preview_entirely() + { + GivenFiles(File(3, @"C:\books\author\book\book.epub", Quality.EPUB, "ebook")); + + var previews = _service.GetConversionPreviews(AuthorId); + + Assert.That(previews, Is.Empty); + } + + [Test] + public void should_import_every_file_of_a_book_as_one_conversion() + { + GivenFiles( + Mp3File(1, @"C:\books\author\book\01.mp3"), + Mp3File(2, @"C:\books\author\book\02.mp3"), + Mp3File(3, @"C:\books\author\book\03.mp3")); + + _service.Execute(new ConvertBookFilesCommand(AuthorId, new List { 1, 2, 3 })); + + Assert.That(_importApprovedBooks.Calls.Count, Is.EqualTo(1)); + Assert.That(_importApprovedBooks.Calls[0].Decisions.Count, Is.EqualTo(3)); + } + + [Test] + public void should_flag_the_import_as_a_library_conversion_that_replaces_existing_files() + { + GivenFiles(Mp3File(1, @"C:\books\author\book\01.mp3")); + + _service.Execute(new ConvertBookFilesCommand(AuthorId, new List { 1 })); + + var call = _importApprovedBooks.Calls.Single(); + + Assert.That(call.ReplaceExisting, Is.True); + Assert.That(call.DownloadClientItem, Is.Null); + Assert.That(call.Decisions.All(d => d.Item.IsLibraryConversion), Is.True); + Assert.That(call.Decisions.All(d => d.Item.Book.Id == BookId), Is.True); + Assert.That(call.Decisions.All(d => d.Item.Edition.Id == EditionId), Is.True); + } + + [Test] + public void should_convert_each_edition_separately() + { + var first = Mp3File(1, @"C:\books\author\book\01.mp3"); + var second = Mp3File(2, @"C:\books\author\other\01.mp3"); + second.EditionId = OtherEditionId; + + GivenFiles(first, second); + + _service.Execute(new ConvertBookFilesCommand(AuthorId, new List { 1, 2 })); + + Assert.That(_importApprovedBooks.Calls.Count, Is.EqualTo(2)); + Assert.That(_importApprovedBooks.Calls.SelectMany(c => c.Decisions).Count(), Is.EqualTo(2)); + } + + [Test] + public void should_convert_the_whole_book_even_when_only_some_of_its_files_were_selected() + { + GivenFiles( + Mp3File(1, @"C:\books\author\book\01.mp3"), + Mp3File(2, @"C:\books\author\book\02.mp3"), + Mp3File(3, @"C:\books\author\book\03.mp3")); + + _service.Execute(new ConvertBookFilesCommand(AuthorId, new List { 1 })); + + var call = _importApprovedBooks.Calls.Single(); + + Assert.That(call.Decisions.Count, Is.EqualTo(3)); + } + + [Test] + public void should_not_pull_in_siblings_that_are_not_convertible() + { + var existingM4b = File(9, @"C:\books\author\book\book.m4b", Quality.M4B); + + GivenFiles( + Mp3File(1, @"C:\books\author\book\01.mp3"), + existingM4b); + + _service.Execute(new ConvertBookFilesCommand(AuthorId, new List { 1 })); + + var call = _importApprovedBooks.Calls.Single(); + + Assert.That(call.Decisions.Count, Is.EqualTo(1)); + Assert.That(call.Decisions[0].Item.Path, Does.EndWith("01.mp3")); + } + + [Test] + public void should_not_import_anything_when_no_file_is_eligible() + { + GivenFiles(File(2, @"C:\books\author\book\book.m4b", Quality.M4B)); + + _service.Execute(new ConvertBookFilesCommand(AuthorId, new List { 2 })); + + Assert.That(_importApprovedBooks.Calls, Is.Empty); + } + + [Test] + public void should_convert_every_eligible_file_for_the_selected_authors() + { + GivenFiles(Mp3File(1, @"C:\books\author\book\01.mp3")); + + _service.Execute(new ConvertAuthorCommand { AuthorIds = new List { AuthorId } }); + + Assert.That(_importApprovedBooks.Calls.Count, Is.EqualTo(1)); + } + + private void GivenFiles(params BookFile[] files) + { + _mediaFileServiceProxy.Files = files.ToList(); + + foreach (var file in files) + { + _diskProviderProxy.ExistingFiles.Add(file.Path); + } + } + + private static BookFile Mp3File(int id, string path) + { + return File(id, path, Quality.MP3); + } + + private static BookFile File(int id, string path, Quality quality, string mediaType = "audiobook") + { + return new BookFile + { + Id = id, + Path = path, + Size = 1024, + Quality = new QualityModel(quality), + EditionId = EditionId, + MediaType = mediaType, + Part = id + }; + } + + private static Author Author(int? convertToQualityId, bool allowM4b = true) + { + var profile = new QualityProfile + { + Id = 10, + Name = "Audiobook", + ProfileType = ProfileType.Audiobook, + ConvertToQualityId = convertToQualityId, + Items = new List + { + new() { Quality = Quality.MP3, Allowed = true }, + new() { Quality = Quality.M4B, Allowed = allowM4b } + } + }; + + return new Author + { + Id = AuthorId, + Name = "Test Author", + AudiobookQualityProfileId = profile.Id, + AudiobookQualityProfile = profile + }; + } + + private class AuthorServiceProxy : DispatchProxy + { + public Author Author { get; set; } + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + switch (targetMethod?.Name) + { + case nameof(IAuthorService.GetAuthor): + return Author; + case nameof(IAuthorService.GetAuthors): + return new List { Author }; + } + + throw new NotImplementedException($"Test proxy does not implement IAuthorService.{targetMethod?.Name}"); + } + } + + private class BookServiceProxy : DispatchProxy + { + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + if (targetMethod?.Name == nameof(IBookService.GetBook)) + { + return new Book { Id = (int)args[0], AuthorId = AuthorId, Title = "Test Book" }; + } + + throw new NotImplementedException($"Test proxy does not implement IBookService.{targetMethod?.Name}"); + } + } + + private class EditionServiceProxy : DispatchProxy + { + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + if (targetMethod?.Name == nameof(IEditionService.GetEdition)) + { + return new Edition { Id = (int)args[0], BookId = BookId, Title = "Test Edition" }; + } + + throw new NotImplementedException($"Test proxy does not implement IEditionService.{targetMethod?.Name}"); + } + } + + private class MediaFileServiceProxy : DispatchProxy + { + public List Files { get; set; } = new(); + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + switch (targetMethod?.Name) + { + case nameof(IMediaFileService.GetFilesByAuthor): + case nameof(IMediaFileService.GetFilesByBook): + return Files; + case nameof(IMediaFileService.GetFilesByEdition): + var editionId = (int)args[0]; + return Files.Where(f => f.EditionId == editionId).ToList(); + case nameof(IMediaFileService.Get) when args?.Length == 1 && args[0] is IEnumerable ids: + var requested = ids.ToHashSet(); + return Files.Where(f => requested.Contains(f.Id)).ToList(); + } + + throw new NotImplementedException($"Test proxy does not implement IMediaFileService.{targetMethod?.Name}"); + } + } + + private sealed class StubImportDecisionMaker : IMakeImportDecision + { + public List> GetImportDecisions( + List bookFiles, + IdentificationOverrides idOverrides, + ImportDecisionMakerInfo itemInfo, + ImportDecisionMakerConfig config, + CancellationToken cancellationToken = default) + { + return bookFiles + .Select(file => new ImportDecision(new LocalBook + { + Path = file.FullName, + Author = idOverrides?.Author, + Book = idOverrides?.Book, + Edition = idOverrides?.Edition, + Quality = new QualityModel(Quality.MP3) + })) + .ToList(); + } + } + + private sealed class StubImportApprovedBooks : IImportApprovedBooks + { + public sealed record Call( + List> Decisions, + bool ReplaceExisting, + NzbDrone.Core.Download.DownloadClientItem DownloadClientItem); + + public List Calls { get; } = new(); + + public List Import( + List> decisions, + bool replaceExisting, + NzbDrone.Core.Download.DownloadClientItem downloadClientItem = null, + ImportMode importMode = ImportMode.Auto, + CancellationToken cancellationToken = default) + { + Calls.Add(new Call(decisions, replaceExisting, downloadClientItem)); + + // Stand in for the conversion the real pipeline performs: one generated file that + // supersedes everything that went into it. + var first = decisions.First().Item; + var converted = new LocalBook + { + Path = first.Path + ".m4b", + Author = first.Author, + Book = first.Book, + Edition = first.Edition, + Quality = new QualityModel(Quality.M4B), + IsGeneratedConversion = true, + IsLibraryConversion = first.IsLibraryConversion, + GeneratedConversionSourcePaths = decisions.Select(d => d.Item.Path).ToList() + }; + + return new List + { + new ImportResult(new ImportDecision(converted), ImportResultType.Imported) + }; + } + } + + private class DiskProviderProxy : DispatchProxy + { + public HashSet ExistingFiles { get; } = new(StringComparer.OrdinalIgnoreCase); + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + if (targetMethod?.Name == nameof(IDiskProvider.FileExists)) + { + return ExistingFiles.Contains((string)args[0]); + } + + if (targetMethod?.Name == nameof(IDiskProvider.GetFileInfo)) + { + return new System.IO.Abstractions.FileSystem().FileInfo.FromFileName((string)args[0]); + } + + throw new NotImplementedException($"Test proxy does not implement IDiskProvider.{targetMethod?.Name}"); + } + } + } +} diff --git a/src/NzbDrone.Core/Localization/Core/en.json b/src/NzbDrone.Core/Localization/Core/en.json index 12c5600a..ae566377 100644 --- a/src/NzbDrone.Core/Localization/Core/en.json +++ b/src/NzbDrone.Core/Localization/Core/en.json @@ -424,9 +424,15 @@ "ConversionSettings": "Conversion Settings", "ConversionSettingsSummary": "Tune audiobook conversion output, threading, and chapter behavior", "ConversionTags": "Conversion Tags", + "Convert": "Convert", + "ConvertBookFiles": "Convert Book Files", "ConvertedWaitingToImport": "Converted - Waiting to import", + "ConvertFiles": "Convert Files", "ConvertMp3ToM4b": "Convert MP3 to M4B", "ConvertMp3ToM4bHelpText": "Before importing, convert downloaded MP3 files to M4B format. Original torrent files follow the same cleanup rules as other downloads: preserve them with Keep seeding permanently or Post-Import Category, otherwise remove them after seeding when Remove Completed Downloads is enabled.", + "ConvertNoEligibleFiles": "None of these files can be converted right now. Set a conversion target on the author's quality profile, and make sure that target is an allowed quality in the profile.", + "ConvertQualityChange": "{sourceQuality} to {targetQuality}", + "ConvertReplacesSourceFilesWarning": "Converting replaces the selected files with the converted file. The originals go to the Recycle Bin if one is configured, and are deleted permanently if not.", "ConvertToFormat": "Convert To Format", "ConvertToQuality": "Convert To", "ConvertToQualityHelpText": "After download, convert compatible files to this format before importing. Original torrent files follow normal cleanup unless protected by Keep seeding permanently on the grabbing indexer or Post-Import Category on the download client. If neither applies, Chaptarr deletes the original torrent files after seeding is done and Remove Completed Downloads is enabled.", @@ -1334,6 +1340,7 @@ "NoName": "Do not show name", "NoPreference": "No Preference", "NoProxiesConfigured": "No proxies configured. Add a proxy if Chaptarr should route indexer or app traffic through one.", + "NoFilesNeedConverting": "No files need converting", "NoResultsFound": "No results found", "NoRecordedMatchExplanation": "No recorded match explanation", "NoTagsHaveBeenAddedYet": "No tags have been added yet. Add tags to link authors with delay profiles, restrictions, or notifications. Click {0} to find out more about tags in Chaptarr.", diff --git a/src/NzbDrone.Core/MediaFiles/BookFileConversionService.cs b/src/NzbDrone.Core/MediaFiles/BookFileConversionService.cs new file mode 100644 index 00000000..bbaf31d9 --- /dev/null +++ b/src/NzbDrone.Core/MediaFiles/BookFileConversionService.cs @@ -0,0 +1,477 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.IO.Abstractions; +using System.Linq; +using NLog; +using NzbDrone.Common.Disk; +using NzbDrone.Common.Extensions; +using NzbDrone.Common.Instrumentation.Extensions; +using NzbDrone.Core.Books; +using NzbDrone.Core.MediaFiles.BookImport; +using NzbDrone.Core.MediaFiles.BookImport.Identification; +using NzbDrone.Core.MediaFiles.Commands; +using NzbDrone.Core.Messaging.Commands; +using NzbDrone.Core.Parser.Model; +using NzbDrone.Core.Qualities; + +namespace NzbDrone.Core.MediaFiles +{ + /// + /// One book file as the conversion preview sees it: what it is now, what it would become, and + /// why it would be left alone. + /// + public class BookFileConversionPreview + { + public int AuthorId { get; set; } + public int BookId { get; set; } + public int EditionId { get; set; } + public int BookFileId { get; set; } + public string BookTitle { get; set; } + public string Path { get; set; } + public long Size { get; set; } + public string SourceQuality { get; set; } + public string TargetQuality { get; set; } + public bool CanConvert { get; set; } + public string Reason { get; set; } + } + + public interface IBookFileConversionService + { + List GetConversionPreviews(int authorId, int? bookId = null); + } + + /// + /// Applies a quality profile's conversion target to files that are already in the library. + /// + /// The conversion itself belongs to the import pipeline, which owns ffmpeg detection, the + /// workspace free-space check, the concurrency semaphore, tagging, chapters, destination naming + /// and the recycle bin. Rather than duplicate any of that, this service hands the existing files + /// back to that pipeline as an import of their own, flagged as a library conversion so the + /// pipeline knows the sources are already-imported files rather than a fresh download. + /// + public class BookFileConversionService : IBookFileConversionService, + IExecute, + IExecute + { + private sealed class ConversionGroup + { + public Author Author { get; init; } + public Book Book { get; init; } + public Edition Edition { get; init; } + public List Files { get; init; } + public Quality TargetQuality { get; init; } + } + + private readonly IAuthorService _authorService; + private readonly IBookService _bookService; + private readonly IEditionService _editionService; + private readonly IMediaFileService _mediaFileService; + private readonly IMakeImportDecision _importDecisionMaker; + private readonly IImportApprovedBooks _importApprovedBooks; + private readonly IDiskProvider _diskProvider; + private readonly Logger _logger; + + public BookFileConversionService(IAuthorService authorService, + IBookService bookService, + IEditionService editionService, + IMediaFileService mediaFileService, + IMakeImportDecision importDecisionMaker, + IImportApprovedBooks importApprovedBooks, + IDiskProvider diskProvider, + Logger logger) + { + _authorService = authorService; + _bookService = bookService; + _editionService = editionService; + _mediaFileService = mediaFileService; + _importDecisionMaker = importDecisionMaker; + _importApprovedBooks = importApprovedBooks; + _diskProvider = diskProvider; + _logger = logger; + } + + public List GetConversionPreviews(int authorId, int? bookId = null) + { + var author = _authorService.GetAuthor(authorId); + if (author == null) + { + return new List(); + } + + var files = bookId.HasValue + ? _mediaFileService.GetFilesByBook(bookId.Value) + : _mediaFileService.GetFilesByAuthor(authorId); + + return BuildPreviews(author, files); + } + + public void Execute(ConvertBookFilesCommand message) + { + var author = _authorService.GetAuthor(message.AuthorId); + if (author == null) + { + _logger.Warn("[CONVERSION] Author {0} not found; nothing to convert", message.AuthorId); + return; + } + + List files; + + if (message.Files?.Count > 0) + { + // Every convertible file of a book becomes one output file, so a selection is read + // as "convert the books these files belong to". Converting a subset would leave the + // book half MP3 and half M4B. + files = ExpandToWholeEditions(author, _mediaFileService.Get(message.Files)); + } + else if (message.BookId.HasValue) + { + files = _mediaFileService.GetFilesByBook(message.BookId.Value); + } + else + { + files = _mediaFileService.GetFilesByAuthor(author.Id); + } + + ConvertFiles(author, files); + } + + public void Execute(ConvertAuthorCommand message) + { + var authors = _authorService.GetAuthors(message.AuthorIds ?? new List()); + + foreach (var author in authors) + { + ConvertFiles(author, _mediaFileService.GetFilesByAuthor(author.Id)); + } + } + + private List ExpandToWholeEditions(Author author, List selected) + { + var editionIds = (selected ?? new List()) + .Where(f => f != null && f.EditionId > 0) + .Select(f => f.EditionId) + .ToHashSet(); + + if (editionIds.Count == 0) + { + return selected ?? new List(); + } + + var expanded = new Dictionary(); + + foreach (var file in selected) + { + expanded[file.Id] = file; + } + + foreach (var editionId in editionIds) + { + foreach (var sibling in _mediaFileService.GetFilesByEdition(editionId)) + { + if (sibling == null || expanded.ContainsKey(sibling.Id)) + { + continue; + } + + // Only pull in siblings that would convert anyway; an eBook or an existing M4B + // filed against the same edition is not part of this conversion. + if (GetConversionTarget(author, sibling, out _) != null) + { + expanded[sibling.Id] = sibling; + } + } + } + + return expanded.Values.ToList(); + } + + private void ConvertFiles(Author author, List files) + { + var groups = BuildConversionGroups(author, files); + + if (groups.Count == 0) + { + _logger.ProgressInfo("No files to convert for {0}", author.Name); + return; + } + + _logger.ProgressInfo("Converting {0} book(s) for {1}", + groups.Count, + author.Name); + + var converted = 0; + var failed = 0; + + for (var i = 0; i < groups.Count; i++) + { + var group = groups[i]; + + _logger.ProgressInfo("Converting '{0}' to {1} ({2}/{3})", + group.Book.Title, + group.TargetQuality.Name, + i + 1, + groups.Count); + + if (ConvertGroup(group)) + { + converted++; + } + else + { + failed++; + } + } + + _logger.ProgressInfo("Converted {0} of {1} book(s) for {2}{3}", + converted, + groups.Count, + author.Name, + failed > 0 ? $"; {failed} failed (see log)" : string.Empty); + } + + private bool ConvertGroup(ConversionGroup group) + { + try + { + var fileInfos = group.Files + .Select(file => _diskProvider.GetFileInfo(file.Path)) + .ToList(); + + var overrides = new IdentificationOverrides + { + Author = group.Author, + Book = group.Book, + Edition = group.Edition + }; + + // itemInfo stays null so the decision maker takes the book and edition given here + // instead of re-matching files that are already correctly matched. + var decisions = _importDecisionMaker.GetImportDecisions( + fileInfos, + overrides, + null, + new ImportDecisionMakerConfig + { + Filter = FilterFilesType.None, + NewDownload = false, + SingleRelease = false, + IncludeExisting = true, + AddNewAuthors = false, + KeepAllEditions = true + }); + + var approved = decisions.Where(d => d.Approved && d.Item != null).ToList(); + + if (approved.Count == 0) + { + _logger.Warn("[CONVERSION] No usable files remained for '{0}'; skipping", group.Book.Title); + return false; + } + + foreach (var decision in approved) + { + decision.Item.IsLibraryConversion = true; + decision.Item.Author = group.Author; + decision.Item.Book = group.Book; + decision.Item.Edition = group.Edition; + } + + var results = _importApprovedBooks.Import(approved, replaceExisting: true); + + var importedConversion = results.Any(r => r.Result == ImportResultType.Imported && + r.ImportDecision?.Item?.IsGeneratedConversion == true); + + if (importedConversion) + { + _logger.Info("[CONVERSION] Converted '{0}' to {1}", group.Book.Title, group.TargetQuality.Name); + return true; + } + + var errors = results + .SelectMany(r => r.Errors ?? new List()) + .Where(e => e.IsNotNullOrWhiteSpace()) + .Distinct() + .ToList(); + + _logger.Warn("[CONVERSION] '{0}' was not converted: {1}", + group.Book.Title, + errors.Any() ? string.Join("; ", errors) : "conversion produced no imported file"); + + return false; + } + catch (Exception ex) + { + _logger.Error(ex, "[CONVERSION] Failed converting '{0}'", group.Book?.Title); + return false; + } + } + + private List BuildConversionGroups(Author author, List files) + { + var groups = new List(); + + foreach (var editionGroup in EligibleFiles(author, files).GroupBy(x => x.File.EditionId)) + { + var edition = _editionService.GetEdition(editionGroup.Key); + if (edition == null) + { + continue; + } + + var book = _bookService.GetBook(edition.BookId); + if (book == null) + { + continue; + } + + edition.Book = book; + + groups.Add(new ConversionGroup + { + Author = author, + Book = book, + Edition = edition, + Files = editionGroup + .Select(x => x.File) + .OrderBy(f => f.Part) + .ThenBy(f => f.Path, StringComparer.OrdinalIgnoreCase) + .ToList(), + TargetQuality = editionGroup.First().Target + }); + } + + return groups + .OrderBy(g => g.Book.Title, StringComparer.OrdinalIgnoreCase) + .ToList(); + } + + private IEnumerable<(BookFile File, Quality Target)> EligibleFiles(Author author, List files) + { + foreach (var file in files ?? new List()) + { + var target = GetConversionTarget(author, file, out _); + + if (target != null) + { + yield return (file, target); + } + } + } + + private List BuildPreviews(Author author, List files) + { + var previews = new List(); + var bookTitles = new Dictionary(); + + foreach (var file in files ?? new List()) + { + var target = GetConversionTarget(author, file, out var reason); + + if (target == null && reason == null) + { + // Nothing about this file is interesting to a conversion preview (an eBook, a + // Calibre-managed file): leave it out rather than listing it as a non-action. + continue; + } + + if (!bookTitles.TryGetValue(file.EditionId, out var bookInfo)) + { + var edition = _editionService.GetEdition(file.EditionId); + var book = edition == null ? null : _bookService.GetBook(edition.BookId); + bookInfo = (book?.Id ?? 0, book?.Title ?? string.Empty); + bookTitles[file.EditionId] = bookInfo; + } + + previews.Add(new BookFileConversionPreview + { + AuthorId = author.Id, + BookId = bookInfo.BookId, + EditionId = file.EditionId, + BookFileId = file.Id, + BookTitle = bookInfo.Title, + Path = file.Path, + Size = file.Size, + SourceQuality = file.Quality?.Quality?.Name ?? Quality.Unknown.Name, + TargetQuality = target?.Name, + CanConvert = target != null, + Reason = reason + }); + } + + return previews + .OrderBy(p => p.BookTitle, StringComparer.OrdinalIgnoreCase) + .ThenBy(p => p.Path, StringComparer.OrdinalIgnoreCase) + .ToList(); + } + + /// + /// The quality this file would be converted to, or null with a reason explaining why it + /// would be left alone. A null target and a null reason together mean the file is simply + /// not a conversion candidate and should not be shown at all. + /// + private Quality GetConversionTarget(Author author, BookFile file, out string reason) + { + reason = null; + + if (file == null || file.Path.IsNullOrWhiteSpace() || file.EditionId <= 0) + { + return null; + } + + var sourceQuality = file.Quality?.Quality ?? Quality.Unknown; + + if (!QualityMediaTypeHelper.IsAudiobookQuality(sourceQuality)) + { + return null; + } + + if (file.CalibreId != 0) + { + reason = "Calibre-managed files are not converted"; + return null; + } + + var target = QualityConversionHelper.GetPlannedConversionTarget(author, file.Quality); + + if (target == null) + { + var profile = author?.GetQualityProfileForQuality(sourceQuality); + + if (profile == null) + { + reason = "No quality profile applies to this file"; + } + else if (!profile.ConvertToQualityId.HasValue || profile.ConvertToQualityId.Value <= 0) + { + reason = $"Quality profile '{profile.Name}' has no conversion target set"; + } + else if (profile.ConvertToQualityId.Value == sourceQuality.Id) + { + reason = $"Already {sourceQuality.Name}"; + } + else + { + reason = $"Conversion target is not allowed by quality profile '{profile.Name}'"; + } + + return null; + } + + if (Path.GetExtension(file.Path).Equals(".m4b", StringComparison.OrdinalIgnoreCase) && + target == Quality.M4B) + { + reason = "Already M4B"; + return null; + } + + if (!_diskProvider.FileExists(file.Path)) + { + reason = "File is missing from disk"; + return null; + } + + return target; + } + } +} diff --git a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs index 9a955cfa..945ba6cb 100644 --- a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs +++ b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs @@ -185,7 +185,7 @@ public List Import( foreach (var md in matchedDecisions) { var local = md.Item; - if (local?.IsManualImport == true || downloadForced) + if (local?.IsManualImport == true || local?.IsLibraryConversion == true || downloadForced) { _logger.Debug("[QUALITY-GATE] Skipping quality gating for explicit user import/grab: '{0}'", local.Path); gatedMatched.Add(md); @@ -342,7 +342,12 @@ public List Import( // CRITICAL BUG FIX: Only consider it a replacement if there are ACTUAL files to replace // If the book is "Missing" (no files), we should import to it, not create a duplicate var filesToReplace = existingFiles.Any() && replaceExisting; - var manualReplaceExisting = replaceExisting && (downloadForced || bookDecisions.Any(d => d.Item?.IsManualImport == true)); + + // A library conversion is an explicit user action against files that are already + // imported, so "these exact files are already imported" is the normal case here and + // must not skip the batch the way it does for an automatic grab. + var isLibraryConversion = bookDecisions.Any(d => d.Item?.IsLibraryConversion == true); + var manualReplaceExisting = replaceExisting && (downloadForced || isLibraryConversion || bookDecisions.Any(d => d.Item?.IsManualImport == true)); _logger.Debug("[UPGRADE-CHECK] replaceExisting={0}, existingFiles.Any()={1}, filesToReplace={2}, UpgradeAllowed={3}", replaceExisting, existingFiles.Any(), filesToReplace, qualityProfile?.UpgradeAllowed ?? true); @@ -1068,9 +1073,28 @@ private void CleanupNewBookInstanceIfEmpty(bool createdNewBookInstance, Book new existingFiles.Count); var manualReplaceExisting = replaceExisting && (localBook.IsManualImport || downloadForced); + + // The files this conversion consumed are the ones it is meant to supersede. They + // are named explicitly rather than swept up by manualReplaceExisting, which takes + // every other file on the book: converting an audiobook must not remove the + // book's eBook, and it must not remove a second audiobook edition either. + var conversionSourcePaths = localBook.IsLibraryConversion && localBook.IsGeneratedConversion + ? (localBook.GeneratedConversionSourcePaths ?? new List()) + .Where(p => p.IsNotNullOrWhiteSpace()) + .ToHashSet(StringComparer.OrdinalIgnoreCase) + : new HashSet(StringComparer.OrdinalIgnoreCase); + var filesToReplace = existingFiles.Where(f => !f.Path.Equals(localBook.Path, StringComparison.OrdinalIgnoreCase) && - (f.EditionId == edition.Id || manualReplaceExisting)).ToList(); + (f.EditionId == edition.Id || manualReplaceExisting || conversionSourcePaths.Contains(f.Path))).ToList(); + + // A library conversion replaces what it converted and nothing else. Same-edition + // files that were not part of the conversion (an eBook filed against the same + // edition, a part that failed to convert) stay where they are. + if (localBook.IsLibraryConversion && localBook.IsGeneratedConversion) + { + filesToReplace = filesToReplace.Where(f => conversionSourcePaths.Contains(f.Path)).ToList(); + } // Relocation is not an "upgrade/replacement"; do not block or stage/delete other files. if (relocateExistingFile != null) @@ -1523,12 +1547,25 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author bool hasRejectedTrackedDownloadDecisions, bool downloadForced) { - if (downloadClientItem == null || bookDecisions == null || bookDecisions.Count == 0) + if (bookDecisions == null || bookDecisions.Count == 0) { return (bookDecisions, null, false, null); } var first = bookDecisions.First().Item; + + // Conversion used to require a tracked download, which left manual imports and + // everything already in the library with no way to reach the converter. The download + // id is only ever used as a correlation key, so synthesise a stable one when there is + // no download client item. Stability matters: it keeps the retained-artifact lookup + // and the .chaptarr-conversions work folder tied to this book instead of growing a + // fresh folder on every attempt. + var conversionId = downloadClientItem?.DownloadId; + if (conversionId.IsNullOrWhiteSpace()) + { + conversionId = BuildLocalConversionId(book, first); + } + var sourceQuality = first?.Quality?.Quality ?? Qualities.Quality.Unknown; var qualityProfile = author.GetQualityProfileForQuality(sourceQuality); var targetQuality = QualityConversionHelper.GetPlannedConversionTarget(author, first?.Quality); @@ -1557,18 +1594,24 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author if (hasRejectedTrackedDownloadDecisions) { var error = "Conversion skipped because one or more files in the completed download did not match a local book. Fix the match failure, then retry import."; - _conversionTrackingService?.Fail(downloadClientItem.DownloadId, error); + _conversionTrackingService?.Fail(conversionId, error); PublishConversionFailed(first, inputFiles, book, author, new QualityModel(Qualities.Quality.M4B), null, error, downloadClientItem); return (bookDecisions, null, true, error); } var convertedQuality = new QualityModel(Qualities.Quality.M4B); - var useDetachedJob = _conversionJobService != null && bookDecisions.All(decision => decision.Item?.IsManualImport != true); + // A detached job hands the finished artifact back through ProcessMonitoredDownloads, + // so it only has a way home when a tracked download owns the import. Manual imports + // and library conversions convert inline on the calling thread instead. + var useDetachedJob = _conversionJobService != null && + downloadClientItem != null && + bookDecisions.All(decision => decision.Item?.IsManualImport != true && + decision.Item?.IsLibraryConversion != true); if (!_m4bConversionService.CanConvert(inputFiles)) { var error = "Conversion to M4B is enabled, but one or more files are not compatible with the M4B converter."; - _conversionTrackingService?.Fail(downloadClientItem.DownloadId, error); + _conversionTrackingService?.Fail(conversionId, error); PublishConversionFailed(first, inputFiles, book, author, convertedQuality, null, error, downloadClientItem); return (bookDecisions, null, true, error); } @@ -1585,7 +1628,7 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author if (finalDestinationFolder.IsNullOrWhiteSpace()) { var error = "Unable to determine destination folder for converted M4B."; - _conversionTrackingService?.Fail(downloadClientItem.DownloadId, error); + _conversionTrackingService?.Fail(conversionId, error); PublishConversionFailed(first, inputFiles, book, author, convertedQuality, null, error, downloadClientItem); return (bookDecisions, null, true, error); } @@ -1593,12 +1636,12 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author var destinationConflict = GetConversionDestinationConflictReason(finalDestinationPath, book, qualityProfile, replaceExisting, downloadForced); if (destinationConflict.IsNotNullOrWhiteSpace()) { - _conversionTrackingService?.Fail(downloadClientItem.DownloadId, destinationConflict); + _conversionTrackingService?.Fail(conversionId, destinationConflict); PublishConversionFailed(first, inputFiles, book, author, convertedQuality, null, destinationConflict, downloadClientItem); return (bookDecisions, null, true, destinationConflict); } - var downloadFolderName = GetSafeConvertedFileName(downloadClientItem.DownloadId ?? Guid.NewGuid().ToString("N")); + var downloadFolderName = GetSafeConvertedFileName(conversionId ?? Guid.NewGuid().ToString("N")); workRoot = Path.Combine(finalDestinationFolder, ".chaptarr-conversions", downloadFolderName); workFolder = Path.Combine(workRoot, Guid.NewGuid().ToString("N")); CleanupExpiredConversionArtifacts(Path.GetDirectoryName(workRoot), TimeSpan.FromDays(7)); @@ -1627,28 +1670,28 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author if (TryFindReusableConversionArtifact(workRoot, inputFiles, convertedQuality, tagOptions, audioBitrate, audioChannels, out var reusableOutputPath)) { - _conversionTrackingService?.Start(downloadClientItem.DownloadId, Qualities.Quality.M4B.Id, Qualities.Quality.M4B.Name, "Using retained M4B"); - _conversionTrackingService?.Progress(downloadClientItem.DownloadId, 97m, "Using retained M4B"); + _conversionTrackingService?.Start(conversionId, Qualities.Quality.M4B.Id, Qualities.Quality.M4B.Name, "Using retained M4B"); + _conversionTrackingService?.Progress(conversionId, 97m, "Using retained M4B"); var reusableLocalBook = CreateGeneratedConversionLocalBook(first, bookDecisions, reusableOutputPath, inputFiles, convertedQuality, tagOptions); _logger.Info("[CONVERSION] Reusing retained converted M4B for '{0}': {1}", book.Title, reusableOutputPath); return (new List> { new ImportDecision(reusableLocalBook) }, workRoot, false, null); } - var existingJob = useDetachedJob ? _conversionJobService.Get(downloadClientItem.DownloadId) : null; + var existingJob = useDetachedJob ? _conversionJobService.Get(conversionId) : null; if (existingJob != null) { if (existingJob.Status == ConversionJobStatus.Queued || existingJob.Status == ConversionJobStatus.Converting || existingJob.Status == ConversionJobStatus.Cancelling) { - _logger.Debug("[CONVERSION] Download {0} already has an in-flight conversion job; leaving import pending.", downloadClientItem.DownloadId); + _logger.Debug("[CONVERSION] Download {0} already has an in-flight conversion job; leaving import pending.", conversionId); return (null, existingJob.WorkRoot ?? workRoot, false, null); } if (existingJob.Status == ConversionJobStatus.ReadyToImport) { var error = "The completed conversion artifact is no longer valid for the current source files. Retry import to rebuild it."; - _conversionJobService.Fail(downloadClientItem.DownloadId, error); + _conversionJobService.Fail(conversionId, error); PublishConversionFailed(first, inputFiles, book, author, convertedQuality, existingJob.OutputPath, error, downloadClientItem); return (bookDecisions, null, true, error); } @@ -1664,7 +1707,7 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author var estimate = EstimateConversionWorkspace(inputFiles); if (!HasEnoughConversionWorkspaceSpace(finalDestinationFolder, estimate, out var freeSpaceError)) { - _conversionTrackingService?.Fail(downloadClientItem.DownloadId, freeSpaceError); + _conversionTrackingService?.Fail(conversionId, freeSpaceError); PublishConversionFailed(first, inputFiles, book, author, convertedQuality, outputPath, freeSpaceError, downloadClientItem); return (bookDecisions, null, true, freeSpaceError); } @@ -1680,13 +1723,13 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author if (sources == null || sources.Count == 0) { var error = "Unable to capture stable source-file identities for M4B conversion."; - _conversionJobService.Fail(downloadClientItem.DownloadId, error); + _conversionJobService.Fail(conversionId, error); return (bookDecisions, workRoot, true, error); } _conversionJobService.Enqueue(new ConversionJobRequest { - DownloadId = downloadClientItem.DownloadId, + DownloadId = conversionId, BookTitle = book.Title, WorkRoot = workRoot, WorkFolder = workFolder, @@ -1716,13 +1759,13 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author var conversionSlotAcquired = false; ConversionResult result; - _conversionTrackingService?.Start(downloadClientItem.DownloadId, Qualities.Quality.M4B.Id, Qualities.Quality.M4B.Name, "Waiting for M4B conversion slot"); - _conversionTrackingService?.RegisterCancellation(downloadClientItem.DownloadId, conversionCancellation); + _conversionTrackingService?.Start(conversionId, Qualities.Quality.M4B.Id, Qualities.Quality.M4B.Name, "Waiting for M4B conversion slot"); + _conversionTrackingService?.RegisterCancellation(conversionId, conversionCancellation); try { conversionSemaphore.Wait(conversionCancellation.Token); conversionSlotAcquired = true; - _conversionTrackingService?.Progress(downloadClientItem.DownloadId, 1m, "Converting to M4B"); + _conversionTrackingService?.Progress(conversionId, 1m, "Converting to M4B"); result = _m4bConversionService.ConvertToM4b(conversionInputFiles, outputPath, new ConversionOptions { @@ -1734,7 +1777,7 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author FfmpegThreads = threadPlan.FfmpegThreads, TagOptions = tagOptions, CancellationToken = conversionCancellation.Token, - ProgressHandler = update => _conversionTrackingService?.Progress(downloadClientItem.DownloadId, update.Progress, update.Message) + ProgressHandler = update => _conversionTrackingService?.Progress(conversionId, update.Progress, update.Message) }); } finally @@ -1754,7 +1797,7 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author error = $"{targetQualityName} conversion was cancelled."; _logger.Info("[CONVERSION] Cancelled {0} conversion for '{1}'. Import stopped.", targetQualityName, book.Title); CleanupConversionWorkFolder(workRoot); - _conversionTrackingService?.Cancelled(downloadClientItem.DownloadId, error); + _conversionTrackingService?.Cancelled(conversionId, error); return (bookDecisions, null, true, error); } @@ -1764,7 +1807,7 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author error = $"{error} Converted file retained at: {outputPath}"; } - _conversionTrackingService?.Fail(downloadClientItem.DownloadId, error); + _conversionTrackingService?.Fail(conversionId, error); PublishConversionFailed(first, inputFiles, book, author, convertedQuality, outputPath, error, downloadClientItem); if (!result.RetainOutputOnFailure) { @@ -1774,12 +1817,12 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author return (bookDecisions, null, true, error); } - _conversionTrackingService?.Progress(downloadClientItem.DownloadId, 97m, "Finalizing M4B"); + _conversionTrackingService?.Progress(conversionId, 97m, "Finalizing M4B"); WriteConversionArtifactManifest(workFolder, outputPath, inputFiles, convertedQuality, tagOptions, audioBitrate, audioChannels); var convertedLocalBook = CreateGeneratedConversionLocalBook(first, bookDecisions, outputPath, inputFiles, convertedQuality, tagOptions); - _conversionTrackingService?.Progress(downloadClientItem.DownloadId, 98m, "Preparing import"); + _conversionTrackingService?.Progress(conversionId, 98m, "Preparing import"); _logger.Info("[CONVERSION] Converted {0} source files to M4B for '{1}': {2}", inputFiles.Length, book.Title, outputPath); return (new List> { new ImportDecision(convertedLocalBook) }, workRoot, false, null); @@ -1790,13 +1833,13 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author var error = $"{targetQualityName} conversion was cancelled."; _logger.Info("[CONVERSION] Cancelled {0} conversion for '{1}'. Import stopped.", targetQualityName, book.Title); CleanupConversionWorkFolder(workRoot); - _conversionTrackingService?.Cancelled(downloadClientItem.DownloadId, error); + _conversionTrackingService?.Cancelled(conversionId, error); return (bookDecisions, null, true, error); } catch (Exception ex) { _logger.Error(ex, "[CONVERSION] Failed converting files to M4B for '{0}'", book.Title); - _conversionTrackingService?.Fail(downloadClientItem.DownloadId, ex.Message); + _conversionTrackingService?.Fail(conversionId, ex.Message); PublishConversionFailed(first, inputFiles, book, author, convertedQuality, outputPath, "M4B conversion failed: " + ex.Message, downloadClientItem); CleanupConversionWorkFolder(workRoot ?? workFolder); return (bookDecisions, null, true, "M4B conversion failed: " + ex.Message); @@ -2359,6 +2402,7 @@ private LocalBook CreateGeneratedConversionLocalBook(LocalBook first, IReadOnlyL Narrator = first.Narrator, IsInitialImport = first.IsInitialImport, IsManualImport = first.IsManualImport, + IsLibraryConversion = first.IsLibraryConversion, MatchProvenance = first.MatchProvenance, CommonTags = first.CommonTags }; @@ -2857,6 +2901,26 @@ private static string GetUniqueFileName(string baseName, string extension, HashS return candidate; } + /// + /// Correlation id for a conversion that no download client owns: a manual import, or a + /// book already in the library. Derived from the book and edition so repeated attempts + /// on the same book line up with the artifact retained by the previous attempt. + /// + private static string BuildLocalConversionId(Book book, LocalBook first) + { + var bookId = book?.Id ?? first?.Book?.Id ?? 0; + var editionId = first?.Edition?.Id ?? 0; + + if (bookId > 0) + { + return string.Format(CultureInfo.InvariantCulture, "chaptarr-local-{0}-{1}", bookId, editionId); + } + + // No persisted book to key on (a manual import that is still creating one). A random + // id keeps the conversion isolated; it just cannot reuse a retained artifact. + return "chaptarr-local-" + Guid.NewGuid().ToString("N"); + } + private static string GetSafeConvertedFileName(string name) { if (string.IsNullOrWhiteSpace(name)) diff --git a/src/NzbDrone.Core/MediaFiles/Commands/ConvertAuthorCommand.cs b/src/NzbDrone.Core/MediaFiles/Commands/ConvertAuthorCommand.cs new file mode 100644 index 00000000..bbcd372a --- /dev/null +++ b/src/NzbDrone.Core/MediaFiles/Commands/ConvertAuthorCommand.cs @@ -0,0 +1,24 @@ +using System.Collections.Generic; +using NzbDrone.Core.Messaging.Commands; + +namespace NzbDrone.Core.MediaFiles.Commands +{ + /// + /// Converts every eligible book file for the selected authors to the conversion target + /// configured on their quality profiles. The bulk counterpart to + /// , scoped like . + /// + public class ConvertAuthorCommand : Command + { + public List AuthorIds { get; set; } + + public override bool SendUpdatesToClient => true; + public override bool RequiresDiskAccess => true; + public override bool IsLongRunning => true; + + public ConvertAuthorCommand() + { + AuthorIds = new List(); + } + } +} diff --git a/src/NzbDrone.Core/MediaFiles/Commands/ConvertBookFilesCommand.cs b/src/NzbDrone.Core/MediaFiles/Commands/ConvertBookFilesCommand.cs new file mode 100644 index 00000000..4f2b47e2 --- /dev/null +++ b/src/NzbDrone.Core/MediaFiles/Commands/ConvertBookFilesCommand.cs @@ -0,0 +1,32 @@ +using System.Collections.Generic; +using NzbDrone.Core.Messaging.Commands; + +namespace NzbDrone.Core.MediaFiles.Commands +{ + /// + /// Converts book files that are already in the library to the conversion target configured on + /// the quality profile. Scoped the same way is: an author, and + /// optionally a single book or an explicit list of book file ids. + /// + public class ConvertBookFilesCommand : Command + { + public int AuthorId { get; set; } + public int? BookId { get; set; } + public List Files { get; set; } + + public override bool SendUpdatesToClient => true; + public override bool RequiresDiskAccess => true; + public override bool IsLongRunning => true; + + public ConvertBookFilesCommand() + { + } + + public ConvertBookFilesCommand(int authorId, List files, int? bookId = null) + { + AuthorId = authorId; + Files = files; + BookId = bookId; + } + } +} diff --git a/src/NzbDrone.Core/Parser/Model/LocalBook.cs b/src/NzbDrone.Core/Parser/Model/LocalBook.cs index c8953322..a07164fe 100644 --- a/src/NzbDrone.Core/Parser/Model/LocalBook.cs +++ b/src/NzbDrone.Core/Parser/Model/LocalBook.cs @@ -42,6 +42,12 @@ public class LocalBook public string Narrator { get; set; } public bool IsInitialImport { get; set; } public bool IsManualImport { get; set; } + + // Set when the file is being re-imported from the library purely so the configured + // conversion target can be applied to a book that is already on disk. It is an explicit + // user action, so it bypasses quality gating the same way a manual import does, but it + // must never widen file replacement to the whole book the way a manual import does. + public bool IsLibraryConversion { get; set; } public MatchProvenance MatchProvenance { get; set; } // Manual import suggestion metadata (V5 match) - must not cause DB side effects during suggestion generation. From a9b8a6ee0253e306f3f40ec406daf979b4ab025e Mon Sep 17 00:00:00 2001 From: Brent Date: Sat, 19 Sep 2026 07:39:45 -0700 Subject: [PATCH 2/5] Fix the convert modal's select-all checkbox rendering The footer select-all checkbox passed a custom className to CheckInput without composing the base input class, so the checkbox lost its size and rendered as a thin sliver. Compose from CheckInput.css the same way the Organize modal does, and fix the property order stylelint flagged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GXPbRG8Geef4zMeMQgpQ8H --- frontend/src/Convert/ConvertPreviewModalContent.css | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frontend/src/Convert/ConvertPreviewModalContent.css b/frontend/src/Convert/ConvertPreviewModalContent.css index 8c4e531e..edaa7ade 100644 --- a/frontend/src/Convert/ConvertPreviewModalContent.css +++ b/frontend/src/Convert/ConvertPreviewModalContent.css @@ -17,8 +17,8 @@ } .bookSelectedContainer { - margin-bottom: 0; margin-right: 10px; + margin-bottom: 0; } .bookTitle { @@ -27,11 +27,11 @@ .selectAllInputContainer { margin-right: auto; - padding-top: 6px; - padding-left: 15px; - font-size: 20px; + line-height: 30px; } .selectAllInput { - margin-bottom: 0; + composes: input from '~Components/Form/CheckInput.css'; + + margin: 0; } From 159b0244cefe5a098748fb328adc96c8449e98e0 Mon Sep 17 00:00:00 2001 From: Brent Date: Sat, 19 Sep 2026 10:50:42 -0700 Subject: [PATCH 3/5] Keep library conversions in the book's existing folder A library conversion wrote the M4B to the folder the naming scheme calls for. When a library predates Chaptarr, that is usually a different folder from the one the MP3s were in, and tools that key books by folder see a new book: Audiobookshelf marked the old item missing and created a new one with no listening progress. Converting in place, which is what Audiobookshelf's own M4B tool does, keeps the same item and the listener's position. Library conversions now write to the folder the source files share, or to their parent when they sit in disc subfolders (CD1, Disc 2), and apply naming to the file name only. Anything that does not form one book folder inside the author's root folder falls back to normal naming. Downloads and manual imports are unchanged, and Organize still moves a book to the naming scheme on request. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GXPbRG8Geef4zMeMQgpQ8H --- .../BookFileMovingServiceFixture.cs | 119 ++++++++++++++++++ .../MediaFiles/BookFileMovingService.cs | 82 ++++++++++++ .../BookImport/ImportApprovedBooks.cs | 12 +- 3 files changed, 210 insertions(+), 3 deletions(-) diff --git a/src/Chaptarr.Core.Test/MediaFiles/BookFileMovingServiceFixture.cs b/src/Chaptarr.Core.Test/MediaFiles/BookFileMovingServiceFixture.cs index 36f678d9..2ec20426 100644 --- a/src/Chaptarr.Core.Test/MediaFiles/BookFileMovingServiceFixture.cs +++ b/src/Chaptarr.Core.Test/MediaFiles/BookFileMovingServiceFixture.cs @@ -535,5 +535,124 @@ public void should_throw_clear_error_when_book_context_cannot_be_rehydrated() Assert.That(exception.Message, Does.Contain("missing book context")); Assert.That(exception.Message, Does.Contain("edition '7'")); } + + private static (BookFileMovingService Service, Author Author, string AuthorFolder, string RootFolder) CreateLibraryConversionScenario() + { + var rootFolder = @"C:\library".AsOsAgnostic(); + var authorFolder = Path.Combine(rootFolder, "H.G. Wells"); + var fileNameBuilder = DispatchProxy.Create(); + ((BuildFileNamesProxy)(object)fileNameBuilder).BookFileNameFactory = + (_, _, _, _) => Path.Combine("The Sleeper Awakes - Alan Munro", "The Sleeper Awakes"); + var authorPathBuilder = DispatchProxy.Create(); + ((AuthorPathBuilderProxy)(object)authorPathBuilder).PathFactory = (_, _) => authorFolder; + var service = CreateService( + fileNameBuilder, + DispatchProxy.Create(), + authorPathBuilder: authorPathBuilder); + + var author = new Author + { + Id = 1, + Name = "H.G. Wells", + AudiobookRootFolderPath = rootFolder, + AudiobookPath = authorFolder + }; + + return (service, author, authorFolder, rootFolder); + } + + private static string ConvertedDestination(BookFileMovingService service, Author author, bool isLibraryConversion, params string[] sourcePaths) + { + var book = new Book { Id = 42, AuthorId = author.Id, Author = author }; + var edition = new Edition { Id = 7, BookId = book.Id, Book = book }; + var bookFile = new BookFile + { + Quality = new QualityModel(Quality.M4B), + MediaType = "audiobook" + }; + var localBook = new NzbDrone.Core.Parser.Model.LocalBook + { + Path = Path.Combine(@"C:\library\H.G. Wells\.chaptarr-conversions\work".AsOsAgnostic(), "The Sleeper Awakes.m4b"), + Author = author, + Book = book, + Edition = edition, + Quality = bookFile.Quality, + IsGeneratedConversion = true, + IsLibraryConversion = isLibraryConversion, + GeneratedConversionSourcePaths = sourcePaths.ToList() + }; + + return service.GetImportDestinationPath(bookFile, localBook); + } + + [Test] + public void library_conversion_should_stay_in_the_folder_its_source_files_occupy() + { + var (service, author, authorFolder, _) = CreateLibraryConversionScenario(); + var existingFolder = Path.Combine(authorFolder, "The Sleeper Awakes (Unabridged) [MP3]"); + + var destination = ConvertedDestination(service, author, true, + Path.Combine(existingFolder, "01.mp3"), + Path.Combine(existingFolder, "02.mp3")); + + Assert.That(destination, Is.EqualTo(Path.Combine(existingFolder, "The Sleeper Awakes.m4b"))); + } + + [Test] + public void library_conversion_should_use_the_book_folder_when_sources_are_in_disc_subfolders() + { + var (service, author, authorFolder, _) = CreateLibraryConversionScenario(); + var bookFolder = Path.Combine(authorFolder, "The Sleeper Awakes"); + + var destination = ConvertedDestination(service, author, true, + Path.Combine(bookFolder, "CD1", "01.mp3"), + Path.Combine(bookFolder, "Disc 2", "01.mp3")); + + Assert.That(destination, Is.EqualTo(Path.Combine(bookFolder, "The Sleeper Awakes.m4b"))); + } + + [Test] + public void library_conversion_should_fall_back_to_naming_when_sources_span_sibling_book_folders() + { + var (service, author, authorFolder, _) = CreateLibraryConversionScenario(); + + var destination = ConvertedDestination(service, author, true, + Path.Combine(authorFolder, "Book A", "01.mp3"), + Path.Combine(authorFolder, "Book B", "01.mp3")); + + Assert.That(destination, Is.EqualTo(Path.Combine(authorFolder, "The Sleeper Awakes - Alan Munro", "The Sleeper Awakes.m4b"))); + } + + [Test] + public void library_conversion_should_fall_back_to_naming_when_sources_are_outside_the_root_folder() + { + var (service, author, authorFolder, _) = CreateLibraryConversionScenario(); + var elsewhere = Path.Combine(@"C:\elsewhere".AsOsAgnostic(), "The Sleeper Awakes"); + + var destination = ConvertedDestination(service, author, true, Path.Combine(elsewhere, "01.mp3")); + + Assert.That(destination, Is.EqualTo(Path.Combine(authorFolder, "The Sleeper Awakes - Alan Munro", "The Sleeper Awakes.m4b"))); + } + + [Test] + public void library_conversion_should_not_write_loose_into_the_root_folder() + { + var (service, author, authorFolder, rootFolder) = CreateLibraryConversionScenario(); + + var destination = ConvertedDestination(service, author, true, Path.Combine(rootFolder, "01.mp3")); + + Assert.That(destination, Is.EqualTo(Path.Combine(authorFolder, "The Sleeper Awakes - Alan Munro", "The Sleeper Awakes.m4b"))); + } + + [Test] + public void download_and_manual_import_conversions_should_still_follow_naming() + { + var (service, author, authorFolder, _) = CreateLibraryConversionScenario(); + var existingFolder = Path.Combine(authorFolder, "The Sleeper Awakes (Unabridged) [MP3]"); + + var destination = ConvertedDestination(service, author, false, Path.Combine(existingFolder, "01.mp3")); + + Assert.That(destination, Is.EqualTo(Path.Combine(authorFolder, "The Sleeper Awakes - Alan Munro", "The Sleeper Awakes.m4b"))); + } } } diff --git a/src/NzbDrone.Core/MediaFiles/BookFileMovingService.cs b/src/NzbDrone.Core/MediaFiles/BookFileMovingService.cs index 605dc0ab..14c835f9 100644 --- a/src/NzbDrone.Core/MediaFiles/BookFileMovingService.cs +++ b/src/NzbDrone.Core/MediaFiles/BookFileMovingService.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Text.RegularExpressions; using NLog; using NzbDrone.Common; using NzbDrone.Common.Disk; @@ -51,6 +52,9 @@ public static BookFileMovePlan Skipped(string reason) public class BookFileMovingService : IMoveBookFiles { + // "CD1", "CD 01", "Disc 2", "Disk 03": a disc of one book rather than a book of its own. + private static readonly Regex DiscFolderRegex = new(@"^(cd|dis[ck])\s*\d{1,3}$", RegexOptions.IgnoreCase | RegexOptions.Compiled); + private readonly IEditionService _editionService; private readonly IUpdateBookFileService _updateBookFileService; private readonly IBuildFileNames _buildFileNames; @@ -275,6 +279,72 @@ public string GetImportDestinationPath(BookFile bookFile, LocalBook localBook) return GetImportDestinationPath(bookFile, localBook, out _); } + /// + /// The folder a library conversion should write its output to: the one folder all source + /// files share, or their parent when they sit in per-disc subfolders (Book/CD1, Book/CD2). + /// Null means "use normal naming": not a library conversion, sources that do not form one + /// book folder, or a folder that is not strictly inside the author's root folder. + /// + private static string GetLibraryConversionFolder(LocalBook localBook, BookFile bookFile) + { + if (localBook?.IsLibraryConversion != true) + { + return null; + } + + var sourceFolders = (localBook.GeneratedConversionSourcePaths ?? new List()) + .Where(path => path.IsNotNullOrWhiteSpace()) + .Select(Path.GetDirectoryName) + .Where(folder => folder.IsNotNullOrWhiteSpace()) + .Distinct(PathEqualityComparer.Instance) + .ToList(); + + if (sourceFolders.Count == 0) + { + return null; + } + + string bookFolder; + + if (sourceFolders.Count == 1) + { + bookFolder = sourceFolders[0]; + } + else + { + // Disc subfolders only: every source folder must be a disc-named direct child of + // one parent, which is how Audiobookshelf and most players read a single book laid + // out as CD1/CD2. Anything else, such as files spread across sibling book folders, + // falls back to naming rather than climbing to the author folder. + if (!sourceFolders.All(folder => DiscFolderRegex.IsMatch(Path.GetFileName(folder) ?? string.Empty))) + { + return null; + } + + var parents = sourceFolders + .Select(Path.GetDirectoryName) + .Distinct(PathEqualityComparer.Instance) + .ToList(); + + if (parents.Count != 1 || parents[0].IsNullOrWhiteSpace()) + { + return null; + } + + bookFolder = parents[0]; + } + + var rootFolder = localBook.Author?.GetRootFolderForQuality(bookFile?.Quality?.Quality); + + // Never write outside the library, and never loose into the root folder itself. + if (rootFolder.IsNullOrWhiteSpace() || !rootFolder.IsParentPath(bookFolder)) + { + return null; + } + + return bookFolder; + } + private string GetImportDestinationPath(BookFile bookFile, LocalBook localBook, out List replicaPaths) { replicaPaths = null; @@ -283,6 +353,18 @@ private string GetImportDestinationPath(BookFile bookFile, LocalBook localBook, var extension = Path.GetExtension(localBook.Path); var fileNameOnly = Path.GetFileName(newFileName) + extension; + // A library conversion changes a book's format, not where it lives. Tools that sit on + // top of the library key books by folder: Audiobookshelf keeps listening progress per + // folder, so a converted file written to a newly named folder shows up there as a new + // book with no progress, and the old one as missing. Keep the converted file in the + // folder its source files already occupy and apply naming to the file name only. + // Organize still moves the book to the naming scheme when the user asks for that. + var libraryConversionFolder = GetLibraryConversionFolder(localBook, bookFile); + if (libraryConversionFolder != null) + { + return Path.Combine(libraryConversionFolder, fileNameOnly); + } + var bookPath = _authorPathBuilder.BuildPathForQuality(localBook.Author, bookFile.Quality.Quality, useExistingRelativeFolder: false); var filePath = Path.Combine(bookPath, newFileName + extension); diff --git a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs index 945ba6cb..0e3af123 100644 --- a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs +++ b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs @@ -1623,7 +1623,7 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author try { - var finalDestinationPath = GetConvertedImportDestinationPath(first, book, author, outputName, convertedQuality); + var finalDestinationPath = GetConvertedImportDestinationPath(first, book, author, outputName, convertedQuality, inputFiles); var finalDestinationFolder = Path.GetDirectoryName(finalDestinationPath); if (finalDestinationFolder.IsNullOrWhiteSpace()) { @@ -2408,7 +2408,7 @@ private LocalBook CreateGeneratedConversionLocalBook(LocalBook first, IReadOnlyL }; } - private string GetConvertedImportDestinationPath(LocalBook source, Book book, Author author, string outputName, QualityModel convertedQuality) + private string GetConvertedImportDestinationPath(LocalBook source, Book book, Author author, string outputName, QualityModel convertedQuality, IReadOnlyCollection inputFiles) { if (source?.Edition == null) { @@ -2430,7 +2430,13 @@ private string GetConvertedImportDestinationPath(LocalBook source, Book book, Au Edition = source.Edition, Quality = convertedQuality, Part = source.Part > 0 ? source.Part : 1, - PartCount = source.PartCount + PartCount = source.PartCount, + + // Carry the library-conversion context so this preview resolves to the same + // folder the mover will actually use, and the destination conflict check and + // work folder are judged against the real destination. + IsLibraryConversion = source.IsLibraryConversion, + GeneratedConversionSourcePaths = inputFiles?.ToList() }; var previewBookFile = new BookFile From d46a25a885e1aaaa4ac94cbb5776932ee5ba741e Mon Sep 17 00:00:00 2001 From: Brent Date: Sat, 19 Sep 2026 23:02:53 -0700 Subject: [PATCH 4/5] Show conversion progress and let a running conversion be cancelled A library conversion ran as a task with a single static message, so a long book gave no sign of how far along it was, and there was no way to stop it: the conversion handler ignored the task's cancellation token, so Cancel in System > Tasks had no effect. The converter already reports its progress to the conversion tracker, but from its own threads, where the task's status message cannot see it. While a book converts, poll the tracker and mirror the percentage into the task message, so it shows in the sidebar and in System > Tasks. The handlers now take the task's cancellation token and pass it to the import, which links it to the converter's own cancellation. Cancelling the task stops m4b-tool, cleans up the work folder, and leaves the book's original files untouched, since nothing is replaced until a conversion finishes. A multi-book run stops before starting the next book. The tracking id is now built by one shared helper, so the import pipeline and the conversion service cannot disagree on it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GXPbRG8Geef4zMeMQgpQ8H --- .../BookFileConversionServiceFixture.cs | 89 ++++++++- .../MediaFiles/BookFileConversionService.cs | 179 +++++++++++++++++- .../BookImport/ImportApprovedBooks.cs | 11 +- .../MediaFiles/LocalConversionId.cs | 17 ++ 4 files changed, 280 insertions(+), 16 deletions(-) create mode 100644 src/NzbDrone.Core/MediaFiles/LocalConversionId.cs diff --git a/src/Chaptarr.Core.Test/MediaFiles/BookFileConversionServiceFixture.cs b/src/Chaptarr.Core.Test/MediaFiles/BookFileConversionServiceFixture.cs index add712fc..095c14f5 100644 --- a/src/Chaptarr.Core.Test/MediaFiles/BookFileConversionServiceFixture.cs +++ b/src/Chaptarr.Core.Test/MediaFiles/BookFileConversionServiceFixture.cs @@ -215,6 +215,69 @@ public void should_not_pull_in_siblings_that_are_not_convertible() Assert.That(call.Decisions[0].Item.Path, Does.EndWith("01.mp3")); } + [Test] + public void should_hand_the_task_cancellation_token_to_the_import() + { + GivenFiles(Mp3File(1, @"C:\books\author\book\01.mp3")); + using var cts = new CancellationTokenSource(); + + _service.Execute(new ConvertBookFilesCommand(AuthorId, new List { 1 }), cts.Token); + + Assert.That(_importApprovedBooks.Calls.Single().CancellationToken, Is.EqualTo(cts.Token)); + } + + [Test] + public void should_not_start_converting_when_the_task_is_already_cancelled() + { + GivenFiles(Mp3File(1, @"C:\books\author\book\01.mp3")); + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + Assert.Throws(() => + _service.Execute(new ConvertBookFilesCommand(AuthorId, new List { 1 }), cts.Token)); + + Assert.That(_importApprovedBooks.Calls, Is.Empty); + } + + [Test] + public void should_stop_the_run_when_cancelled_while_a_book_is_converting() + { + var first = Mp3File(1, @"C:\books\author\book\01.mp3"); + var second = Mp3File(2, @"C:\books\author\other\01.mp3"); + second.EditionId = OtherEditionId; + GivenFiles(first, second); + + using var cts = new CancellationTokenSource(); + _importApprovedBooks.OnImport = () => cts.Cancel(); + + Assert.Throws(() => + _service.Execute(new ConvertBookFilesCommand(AuthorId, new List { 1, 2 }), cts.Token)); + + Assert.That(_importApprovedBooks.Calls.Count, Is.EqualTo(1), "the second book must not start"); + } + + [Test] + public void should_clear_the_tracked_conversion_status_when_a_book_finishes() + { + var tracking = DispatchProxy.Create(); + var service = new BookFileConversionService( + _authorService, + DispatchProxy.Create(), + DispatchProxy.Create(), + _mediaFileService, + new StubImportDecisionMaker(), + _importApprovedBooks, + _diskProvider, + LogManager.GetCurrentClassLogger(), + tracking); + GivenFiles(Mp3File(1, @"C:\books\author\book\01.mp3")); + + service.Execute(new ConvertBookFilesCommand(AuthorId, new List { 1 })); + + Assert.That(((ConversionTrackingProxy)(object)tracking).Cleared, + Is.EqualTo(new List { LocalConversionId.For(BookId, EditionId) })); + } + [Test] public void should_not_import_anything_when_no_file_is_eligible() { @@ -382,9 +445,11 @@ private sealed class StubImportApprovedBooks : IImportApprovedBooks public sealed record Call( List> Decisions, bool ReplaceExisting, - NzbDrone.Core.Download.DownloadClientItem DownloadClientItem); + NzbDrone.Core.Download.DownloadClientItem DownloadClientItem, + CancellationToken CancellationToken); public List Calls { get; } = new(); + public Action OnImport { get; set; } public List Import( List> decisions, @@ -393,7 +458,8 @@ public List Import( ImportMode importMode = ImportMode.Auto, CancellationToken cancellationToken = default) { - Calls.Add(new Call(decisions, replaceExisting, downloadClientItem)); + Calls.Add(new Call(decisions, replaceExisting, downloadClientItem, cancellationToken)); + OnImport?.Invoke(); // Stand in for the conversion the real pipeline performs: one generated file that // supersedes everything that went into it. @@ -417,6 +483,25 @@ public List Import( } } + private class ConversionTrackingProxy : DispatchProxy + { + public List Cleared { get; } = new(); + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + switch (targetMethod?.Name) + { + case nameof(IConversionTrackingService.Clear): + Cleared.Add((string)args[0]); + return null; + case nameof(IConversionTrackingService.Get): + return null; + } + + throw new NotImplementedException($"Test proxy does not implement IConversionTrackingService.{targetMethod?.Name}"); + } + } + private class DiskProviderProxy : DispatchProxy { public HashSet ExistingFiles { get; } = new(StringComparer.OrdinalIgnoreCase); diff --git a/src/NzbDrone.Core/MediaFiles/BookFileConversionService.cs b/src/NzbDrone.Core/MediaFiles/BookFileConversionService.cs index bbaf31d9..fce920bf 100644 --- a/src/NzbDrone.Core/MediaFiles/BookFileConversionService.cs +++ b/src/NzbDrone.Core/MediaFiles/BookFileConversionService.cs @@ -3,6 +3,8 @@ using System.IO; using System.IO.Abstractions; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using NLog; using NzbDrone.Common.Disk; using NzbDrone.Common.Extensions; @@ -13,6 +15,7 @@ using NzbDrone.Core.MediaFiles.Commands; using NzbDrone.Core.Messaging.Commands; using NzbDrone.Core.Parser.Model; +using NzbDrone.Core.ProgressMessaging; using NzbDrone.Core.Qualities; namespace NzbDrone.Core.MediaFiles @@ -70,8 +73,11 @@ private sealed class ConversionGroup private readonly IMakeImportDecision _importDecisionMaker; private readonly IImportApprovedBooks _importApprovedBooks; private readonly IDiskProvider _diskProvider; + private readonly IConversionTrackingService _conversionTrackingService; private readonly Logger _logger; + private static readonly TimeSpan ProgressPollInterval = TimeSpan.FromSeconds(2); + public BookFileConversionService(IAuthorService authorService, IBookService bookService, IEditionService editionService, @@ -79,7 +85,8 @@ public BookFileConversionService(IAuthorService authorService, IMakeImportDecision importDecisionMaker, IImportApprovedBooks importApprovedBooks, IDiskProvider diskProvider, - Logger logger) + Logger logger, + IConversionTrackingService conversionTrackingService = null) { _authorService = authorService; _bookService = bookService; @@ -88,6 +95,7 @@ public BookFileConversionService(IAuthorService authorService, _importDecisionMaker = importDecisionMaker; _importApprovedBooks = importApprovedBooks; _diskProvider = diskProvider; + _conversionTrackingService = conversionTrackingService; _logger = logger; } @@ -107,6 +115,13 @@ public List GetConversionPreviews(int authorId, int? } public void Execute(ConvertBookFilesCommand message) + { + Execute(message, CancellationToken.None); + } + + // The command executor prefers this overload when a handler has one, and cancels the token + // when the task is cancelled from System > Tasks. + public void Execute(ConvertBookFilesCommand message, CancellationToken cancellationToken) { var author = _authorService.GetAuthor(message.AuthorId); if (author == null) @@ -133,16 +148,22 @@ public void Execute(ConvertBookFilesCommand message) files = _mediaFileService.GetFilesByAuthor(author.Id); } - ConvertFiles(author, files); + ConvertFiles(author, files, cancellationToken); } public void Execute(ConvertAuthorCommand message) + { + Execute(message, CancellationToken.None); + } + + public void Execute(ConvertAuthorCommand message, CancellationToken cancellationToken) { var authors = _authorService.GetAuthors(message.AuthorIds ?? new List()); foreach (var author in authors) { - ConvertFiles(author, _mediaFileService.GetFilesByAuthor(author.Id)); + cancellationToken.ThrowIfCancellationRequested(); + ConvertFiles(author, _mediaFileService.GetFilesByAuthor(author.Id), cancellationToken); } } @@ -186,7 +207,7 @@ private List ExpandToWholeEditions(Author author, List selec return expanded.Values.ToList(); } - private void ConvertFiles(Author author, List files) + private void ConvertFiles(Author author, List files, CancellationToken cancellationToken) { var groups = BuildConversionGroups(author, files); @@ -205,15 +226,31 @@ private void ConvertFiles(Author author, List files) for (var i = 0; i < groups.Count; i++) { + cancellationToken.ThrowIfCancellationRequested(); + var group = groups[i]; + var position = $"({i + 1}/{groups.Count})"; - _logger.ProgressInfo("Converting '{0}' to {1} ({2}/{3})", + _logger.ProgressInfo("Converting '{0}' to {1} {2}", group.Book.Title, group.TargetQuality.Name, - i + 1, - groups.Count); + position); - if (ConvertGroup(group)) + var succeeded = ConvertGroup(group, position, cancellationToken); + + if (cancellationToken.IsCancellationRequested) + { + // The converter was stopped part-way. Nothing is replaced until a conversion + // finishes, so this book's original files are untouched. + _logger.Info("[CONVERSION] Cancelled while converting '{0}'; its original files were left as they were. {1} of {2} book(s) had been converted.", + group.Book.Title, + converted, + groups.Count); + + throw new OperationCanceledException(cancellationToken); + } + + if (succeeded) { converted++; } @@ -230,8 +267,11 @@ private void ConvertFiles(Author author, List files) failed > 0 ? $"; {failed} failed (see log)" : string.Empty); } - private bool ConvertGroup(ConversionGroup group) + private bool ConvertGroup(ConversionGroup group, string position, CancellationToken cancellationToken) { + var conversionId = LocalConversionId.For(group.Book.Id, group.Edition.Id); + using var progressReporter = StartProgressReporter(group, position, conversionId); + try { var fileInfos = group.Files @@ -277,7 +317,7 @@ private bool ConvertGroup(ConversionGroup group) decision.Item.Edition = group.Edition; } - var results = _importApprovedBooks.Import(approved, replaceExisting: true); + var results = _importApprovedBooks.Import(approved, replaceExisting: true, cancellationToken: cancellationToken); var importedConversion = results.Any(r => r.Result == ImportResultType.Imported && r.ImportDecision?.Item?.IsGeneratedConversion == true); @@ -288,6 +328,12 @@ private bool ConvertGroup(ConversionGroup group) return true; } + if (cancellationToken.IsCancellationRequested) + { + // Not a failure: the caller reports the cancellation and stops the run. + return false; + } + var errors = results .SelectMany(r => r.Errors ?? new List()) .Where(e => e.IsNotNullOrWhiteSpace()) @@ -300,11 +346,124 @@ private bool ConvertGroup(ConversionGroup group) return false; } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + // Reported by the caller, which stops the run. + return false; + } catch (Exception ex) { _logger.Error(ex, "[CONVERSION] Failed converting '{0}'", group.Book?.Title); return false; } + finally + { + // Library conversions have no queue item to retire their status, so drop it here. + _conversionTrackingService?.Clear(conversionId); + } + } + + /// + /// The converter reports its progress to the conversion tracker from its own threads, which + /// the task's status message cannot see. Poll the tracker while a book converts and mirror + /// the percentage into this task's message, so it shows in the sidebar and in + /// System > Tasks. + /// + private IDisposable StartProgressReporter(ConversionGroup group, string position, string conversionId) + { + if (_conversionTrackingService == null) + { + return null; + } + + var command = ProgressMessageContext.CommandModel; + var lastReported = -1; + + return new PollingReporter(ProgressPollInterval, () => + { + var progress = _conversionTrackingService.Get(conversionId)?.Progress; + if (!progress.HasValue) + { + return; + } + + var percent = (int)Math.Floor(progress.Value); + if (percent == lastReported) + { + return; + } + + lastReported = percent; + + // This runs on a pool thread. Attribute the message to the conversion's task for the + // duration of this call only, so nothing else that later runs on the thread writes + // into that task's status. + var previous = ProgressMessageContext.CommandModel; + ProgressMessageContext.CommandModel = command; + + try + { + _logger.ProgressInfo("Converting '{0}' to {1} {2} - {3}%", + group.Book.Title, + group.TargetQuality.Name, + position, + percent); + } + finally + { + ProgressMessageContext.CommandModel = previous; + } + }); + } + + private sealed class PollingReporter : IDisposable + { + private readonly CancellationTokenSource _stop = new(); + private readonly Task _loop; + + public PollingReporter(TimeSpan interval, Action report) + { + _loop = Task.Run(async () => + { + while (!_stop.IsCancellationRequested) + { + try + { + await Task.Delay(interval, _stop.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + break; + } + + try + { + report(); + } + catch + { + // Progress is best-effort; it must never disturb the conversion. + } + } + }); + } + + public void Dispose() + { + _stop.Cancel(); + + try + { + // Let an in-flight report finish, so it cannot overwrite the next status message. + _loop.Wait(TimeSpan.FromSeconds(5)); + } + catch + { + // Nothing to do: the loop only ever ends by cancellation. + } + + _stop.Dispose(); + } } private List BuildConversionGroups(Author author, List files) diff --git a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs index 0e3af123..d49cea1d 100644 --- a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs +++ b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs @@ -352,7 +352,7 @@ public List Import( _logger.Debug("[UPGRADE-CHECK] replaceExisting={0}, existingFiles.Any()={1}, filesToReplace={2}, UpgradeAllowed={3}", replaceExisting, existingFiles.Any(), filesToReplace, qualityProfile?.UpgradeAllowed ?? true); - var conversionResult = ConvertBookGroupIfNeeded(bookDecisions, book, author, downloadClientItem, replaceExisting, hasRejectedTrackedDownloadDecisions, downloadForced); + var conversionResult = ConvertBookGroupIfNeeded(bookDecisions, book, author, downloadClientItem, replaceExisting, hasRejectedTrackedDownloadDecisions, downloadForced, cancellationToken); conversionWorkFolder = conversionResult.WorkFolder; if (!conversionResult.Failed && conversionResult.Decisions == null) { @@ -1545,7 +1545,8 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author DownloadClientItem downloadClientItem, bool replaceExisting, bool hasRejectedTrackedDownloadDecisions, - bool downloadForced) + bool downloadForced, + CancellationToken cancellationToken) { if (bookDecisions == null || bookDecisions.Count == 0) { @@ -1754,7 +1755,9 @@ private string GetCustomFormatImportRejectionReason(LocalBook localBook, Author } var threadPlan = GetAudiobookConversionThreadPlan(inputFiles.Length); - using var conversionCancellation = new CancellationTokenSource(); + // Linked to the caller's token so cancelling the command that asked for this + // conversion (a library conversion's task) stops the running converter too. + using var conversionCancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); var conversionSemaphore = GetAudiobookConversionSemaphore(); var conversionSlotAcquired = false; ConversionResult result; @@ -2919,7 +2922,7 @@ private static string BuildLocalConversionId(Book book, LocalBook first) if (bookId > 0) { - return string.Format(CultureInfo.InvariantCulture, "chaptarr-local-{0}-{1}", bookId, editionId); + return LocalConversionId.For(bookId, editionId); } // No persisted book to key on (a manual import that is still creating one). A random diff --git a/src/NzbDrone.Core/MediaFiles/LocalConversionId.cs b/src/NzbDrone.Core/MediaFiles/LocalConversionId.cs new file mode 100644 index 00000000..9968ecc2 --- /dev/null +++ b/src/NzbDrone.Core/MediaFiles/LocalConversionId.cs @@ -0,0 +1,17 @@ +using System.Globalization; + +namespace NzbDrone.Core.MediaFiles +{ + /// + /// The correlation id used to track a conversion that no download client owns: a manual + /// import, or a book already in the library. Shared so the import pipeline that runs the + /// conversion and the library conversion service that reports on it agree on one id. + /// + public static class LocalConversionId + { + public static string For(int bookId, int editionId) + { + return string.Format(CultureInfo.InvariantCulture, "chaptarr-local-{0}-{1}", bookId, editionId); + } + } +} From 1d0ad03c876ade546a7e2a69581fe340aa781f1d Mon Sep 17 00:00:00 2001 From: Brent Date: Mon, 5 Oct 2026 08:42:52 -0700 Subject: [PATCH 5/5] Check the converted file's real destination before converting A multi-file conversion is merged into one M4B that has no part number, so the import names it "Title.m4b". The destination preview copied the first source file's part fields (1 of N), so it was named "Title - 01.m4b" (or "Title (1).m4b"). The conflict check before conversion looked at that path, found nothing, and a taken destination was only found after a full conversion. The preview now carries the same part values as the converted file. The check finds the real destination before any conversion runs, and the work folder output gets the final name. Fixes #284 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GXPbRG8Geef4zMeMQgpQ8H --- ...mportApprovedBooksAdditionalCopyFixture.cs | 132 +++++++++++++++++- .../BookImport/ImportApprovedBooks.cs | 13 +- 2 files changed, 139 insertions(+), 6 deletions(-) diff --git a/src/Chaptarr.Core.Test/MediaFiles/ImportApprovedBooksAdditionalCopyFixture.cs b/src/Chaptarr.Core.Test/MediaFiles/ImportApprovedBooksAdditionalCopyFixture.cs index 77715e8b..39417824 100644 --- a/src/Chaptarr.Core.Test/MediaFiles/ImportApprovedBooksAdditionalCopyFixture.cs +++ b/src/Chaptarr.Core.Test/MediaFiles/ImportApprovedBooksAdditionalCopyFixture.cs @@ -138,6 +138,10 @@ public void Delete(BookFile bookFile, DeleteMediaFileReason reason) private sealed class StubMoveBookFiles : IMoveBookFiles { public string DestinationPath { get; set; } + + // When set, names destinations from the BookFile, the way the file name builder does, + // instead of returning DestinationPath for everything. + public Func DestinationFor { get; set; } public int MoveCalls { get; private set; } public int CopyCalls { get; private set; } public int PreviewCalls { get; private set; } @@ -155,7 +159,7 @@ public BookFile MoveBookFile(BookFile bookFile, Author author, BookFileMovePlan public BookFile MoveBookFile(BookFile bookFile, LocalBook localBook) { MoveCalls++; - bookFile.Path = DestinationPath ?? bookFile.Path; + bookFile.Path = DestinationFor?.Invoke(bookFile) ?? DestinationPath ?? bookFile.Path; TransferFile(localBook.Path, bookFile.Path, copy: false); return bookFile; } @@ -163,7 +167,7 @@ public BookFile MoveBookFile(BookFile bookFile, LocalBook localBook) public BookFile CopyBookFile(BookFile bookFile, LocalBook localBook) { CopyCalls++; - bookFile.Path = DestinationPath ?? bookFile.Path; + bookFile.Path = DestinationFor?.Invoke(bookFile) ?? DestinationPath ?? bookFile.Path; TransferFile(localBook.Path, bookFile.Path, copy: true); return bookFile; } @@ -171,7 +175,7 @@ public BookFile CopyBookFile(BookFile bookFile, LocalBook localBook) public string GetImportDestinationPath(BookFile bookFile, LocalBook localBook) { PreviewCalls++; - return DestinationPath; + return DestinationFor?.Invoke(bookFile) ?? DestinationPath; } private void TransferFile(string sourcePath, string destinationPath, bool copy) @@ -2065,6 +2069,128 @@ public void should_skip_conversion_when_converted_destination_already_exists() } } + // #284: a multi-file download is merged into one M4B with no part number, so the check + // before conversion has to look at "Black Sheep.m4b", not the first part's "Black Sheep - 01.m4b". + [Test] + public void should_find_a_taken_destination_before_converting_a_multi_part_download() + { + var sourceDir = Path.Combine(TestContext.CurrentContext.WorkDirectory, $"multi-part-conflict-{Guid.NewGuid():N}"); + var destinationDir = Path.Combine(TestContext.CurrentContext.WorkDirectory, $"conversion-destination-{Guid.NewGuid():N}"); + Directory.CreateDirectory(sourceDir); + Directory.CreateDirectory(destinationDir); + File.WriteAllText(Path.Combine(destinationDir, "Black Sheep.m4b"), "already imported m4b"); + + try + { + var (_, author, book, edition) = CreateAudiobookConversionGraph(24); + var conversion = new SuccessfulM4bConversionService(); + var service = CreateServiceForPartNaming(edition, conversion, destinationDir); + + var results = service.Import( + MultiPartDecisions(sourceDir, author, book, edition, partCount: 3), + replaceExisting: false, + downloadClientItem: new DownloadClientItem { DownloadId = "multi-part-conflict" }, + importMode: ImportMode.Move, + cancellationToken: CancellationToken.None); + + Assert.That(conversion.ConvertCalls, Is.EqualTo(0), "nothing is converted when the destination is taken"); + Assert.That(results.Select(r => r.Result), Is.All.EqualTo(ImportResultType.Skipped)); + Assert.That(results.SelectMany(r => r.Errors).Distinct().Single(), + Does.Contain("untracked file already exists at the destination").And.Contain("Black Sheep.m4b")); + Assert.That(Directory.Exists(Path.Combine(destinationDir, ".chaptarr-conversions")), Is.False); + } + finally + { + Directory.Delete(sourceDir, recursive: true); + Directory.Delete(destinationDir, recursive: true); + } + } + + [Test] + public void should_name_a_merged_multi_part_conversion_without_a_part_suffix() + { + var sourceDir = Path.Combine(TestContext.CurrentContext.WorkDirectory, $"multi-part-name-{Guid.NewGuid():N}"); + var destinationDir = Path.Combine(TestContext.CurrentContext.WorkDirectory, $"conversion-destination-{Guid.NewGuid():N}"); + Directory.CreateDirectory(sourceDir); + Directory.CreateDirectory(destinationDir); + + try + { + var (_, author, book, edition) = CreateAudiobookConversionGraph(25); + var conversion = new FailingM4bConversionService(); + var service = CreateServiceForPartNaming(edition, conversion, destinationDir); + + service.Import( + MultiPartDecisions(sourceDir, author, book, edition, partCount: 3), + replaceExisting: false, + downloadClientItem: new DownloadClientItem { DownloadId = "multi-part-name" }, + importMode: ImportMode.Move, + cancellationToken: CancellationToken.None); + + Assert.That(conversion.ConvertCalls, Is.EqualTo(1)); + Assert.That(Path.GetFileName(conversion.LastOutputFile), Is.EqualTo("Black Sheep.m4b")); + } + finally + { + Directory.Delete(sourceDir, recursive: true); + Directory.Delete(destinationDir, recursive: true); + } + } + + // Names files the way the file name builder does with a {PartNumber} token: a suffix only + // when the file is one part of several. + private static ImportApprovedBooks CreateServiceForPartNaming(Edition edition, IM4bConversionService conversion, string destinationDir) + { + var mover = new StubMoveBookFiles + { + DestinationFor = bookFile => Path.Combine( + destinationDir, + bookFile.PartCount > 1 ? $"Black Sheep - {bookFile.Part:00}{Path.GetExtension(bookFile.Path)}" : $"Black Sheep{Path.GetExtension(bookFile.Path)}") + }; + + return new ImportApprovedBooks( + new StubMediaFileService(), + new StubMetadataTagService(), + Proxy(), + Proxy(), + Proxy(), + CreateEditionService(new List { edition }), + Proxy(), + Proxy(), + mover, + Proxy(), + Proxy(), + new NoOpEventAggregator(), + Proxy(), + Proxy(), + Proxy(), + Proxy(), + conversion, + LogManager.GetLogger("ImportApprovedBooksAdditionalCopyFixture")); + } + + private static List> MultiPartDecisions(string sourceDir, Author author, Book book, Edition edition, int partCount) + { + return Enumerable.Range(1, partCount) + .Select(part => + { + var path = Path.Combine(sourceDir, $"{part:00}.mp3"); + File.WriteAllText(path, "fake mp3"); + + return new ImportDecision(new LocalBook + { + Path = path, + Book = book, + Author = author, + Edition = edition, + Part = part, + PartCount = partCount, + Quality = new QualityModel { Quality = Quality.MP3, Revision = new Revision() } + }); + }) + .ToList(); + } + [Test] public void should_skip_conversion_when_converted_destination_is_unmapped() { diff --git a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs index d49cea1d..6f66b3f5 100644 --- a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs +++ b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs @@ -2432,8 +2432,13 @@ private string GetConvertedImportDestinationPath(LocalBook source, Book book, Au Book = book, Edition = source.Edition, Quality = convertedQuality, - Part = source.Part > 0 ? source.Part : 1, - PartCount = source.PartCount, + + // No Part or PartCount. The converted file is a single M4B, and + // CreateGeneratedConversionLocalBook gives it neither, so it is named without a + // part suffix. Copying the first source file's "1 of N" here named the preview + // "Title - 01.m4b" while the import wrote "Title.m4b": the conflict check looked + // at the wrong path, and a taken destination was only found after a full + // conversion (#284). // Carry the library-conversion context so this preview resolves to the same // folder the mover will actually use, and the destination conflict check and @@ -2449,7 +2454,9 @@ private string GetConvertedImportDestinationPath(LocalBook source, Book book, Au EditionId = source.Edition.Id, Edition = source.Edition, Author = author, - Part = previewLocalBook.Part, + + // The same part values the import gives the converted file's BookFile. + Part = previewLocalBook.Part > 0 ? previewLocalBook.Part : 1, PartCount = previewLocalBook.PartCount, MediaType = BookFile.DetermineMediaType(convertedQuality) };