diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua index b9cb90687..cf8696f18 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua @@ -58,6 +58,8 @@ local DEFAULT_GRID_COLUMNS = CatalogUtil.DEFAULT_GRID_COLUMNS local DEFAULT_GRID_ROWS = CatalogUtil.DEFAULT_GRID_ROWS local MAX_RECENT_SEARCHES = CatalogUtil.MAX_RECENT_SEARCHES local ON_DEVICE_MAX_IDS = CatalogUtil.ON_DEVICE_MAX_IDS +local NOT_ON_DEVICE_SCAN_SIZE = CatalogUtil.NOT_ON_DEVICE_SCAN_SIZE +local scanNotOnDeviceIds = CatalogUtil.scanNotOnDeviceIds local SORTS = CatalogUtil.SORTS local SORT_LABELS = CatalogUtil.SORT_LABELS local NATURAL_ORDER = CatalogUtil.NATURAL_ORDER @@ -236,6 +238,7 @@ function BookOrbitCatalog:init() self.settings = self.settings or {} CatalogWidgets.setAssetIconFiles({ on_device = self:pluginAssetFile("bookorbit.on-device.svg"), + download = self:pluginAssetFile(DOWNLOAD_ICON_FILE), shuffle = self:pluginAssetFile("bookorbit.shuffle.svg"), in_progress = self:pluginAssetFile("bookorbit.in-progress.svg"), collections = self:pluginAssetFile("bookorbit.collections.svg"), @@ -713,9 +716,13 @@ function BookOrbitCatalog:rootItems() end table.insert(items, { text = _("On device"), - mandatory = on_device_count > 0 and tostring(on_device_count) or nil, + mandatory = on_device_count > 0 and CatalogUtil.formatCount(on_device_count) or nil, kind = "on-device", }) + table.insert(items, { + text = _("Not on device"), + kind = "not-on-device", + }) return items end @@ -1006,27 +1013,33 @@ function BookOrbitCatalog:paramsForEntry(section, entry) return params end +-- Assumes an already-wrapped Trapper coroutine, so a caller that has its own +-- requests to make first can reuse it without nesting another wrap. +function BookOrbitCatalog:fetchBookPage(params, title, push) + local query = cloneParams(params) + query.page = query.page or 1 + query.size = self:itemsPerPage() + query.sort = query.sort or self.default_sort or "recently_added" + query.order = self:effectiveOrder(query) + + local body, err = self:fetch(_("Loading books..."), function() + return self.client:catalogBooks(query) + end) + if not body then + if err ~= "cancelled" then + self:showRetry(err, function() + self:loadBooks(params, title, push) + end) + end + return + end + + self:showBookPage(body, query, title or _("Books"), push ~= false) +end + function BookOrbitCatalog:loadBooks(params, title, push) self:runConnected(function() - local query = cloneParams(params) - query.page = query.page or 1 - query.size = self:itemsPerPage() - query.sort = query.sort or self.default_sort or "recently_added" - query.order = self:effectiveOrder(query) - - local body, err = self:fetch(_("Loading books..."), function() - return self.client:catalogBooks(query) - end) - if not body then - if err ~= "cancelled" then - self:showRetry(err, function() - self:loadBooks(params, title, push) - end) - end - return - end - - self:showBookPage(body, query, title or _("Books"), push ~= false) + self:fetchBookPage(params, title, push) end) end @@ -1048,6 +1061,55 @@ function BookOrbitCatalog:loadOnDevice() self:loadBooks({ ids = table.concat(capped, ","), sort = "title" }, _("On device"), true) end +function BookOrbitCatalog:collectNotOnDeviceIds() + return scanNotOnDeviceIds(function(page) + return self:fetch(_("Finding books not on device..."), function() + return self.client:catalogBooks({ + page = page, + size = NOT_ON_DEVICE_SCAN_SIZE, + sort = "recently_added", + order = "desc", + }) + end) + end, function(book) + return self:isOnDevice(book) + end) +end + +function BookOrbitCatalog:loadNotOnDevice() + self:runConnected(function() + -- Without the maps every book reads as absent, which would list books + -- that are already here. refreshOnDevice starts the scan; the list is + -- worth waiting for rather than showing wrong. + if not self:refreshOnDevice() then + UIManager:show(InfoMessage:new{ + text = _("Still checking which books are on this device. Try again in a moment."), + timeout = 3, + }) + return + end + local ids, err = self:collectNotOnDeviceIds() + if not ids then + if err ~= "cancelled" then + self:showRetry(err, function() + self:loadNotOnDevice() + end) + end + return + end + if #ids == 0 then + UIManager:show(InfoMessage:new{ + text = _("The newest books in your library are already on this device."), + timeout = 3, + }) + return + end + -- The tile says "Not on device"; the list says what it actually holds, + -- since the walk covers recent additions rather than the whole library. + self:fetchBookPage({ ids = table.concat(ids, ","), sort = "recently_added" }, _("Not on device (recent)"), true) + end) +end + function BookOrbitCatalog:scopeParams(query) local params = cloneParams(query) params.page = nil @@ -2224,6 +2286,8 @@ function BookOrbitCatalog:onMenuSelect(item) end elseif item.kind == "on-device" then self:loadOnDevice() + elseif item.kind == "not-on-device" then + self:loadNotOnDevice() elseif item.kind == "books" then self:loadBooks(item.params or {}, item.list_title or item.text) elseif item.kind == "book" then diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_bulk_download.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_bulk_download.lua index 1c98e7d58..da087465d 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_bulk_download.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_bulk_download.lua @@ -33,6 +33,7 @@ local cloneParams = CatalogUtil.cloneParams local formatBytes = CatalogUtil.formatBytes local isSupportedFormat = CatalogUtil.isSupportedFormat local safeFilenameBase = CatalogUtil.safeFilenameBase +local shortPath = CatalogUtil.shortPath local shortText = CatalogUtil.shortText local STEP_DELAY = 0.15 @@ -40,6 +41,7 @@ local NEXT_ITEM_DELAY = 0.02 local PROGRESS_THROTTLE = 1 local FAILED_TITLES_LIMIT = 8 local TITLE_LINE_LENGTH = 38 +local FOLDER_LABEL_LENGTH = 32 local MANIFEST_FEATURE = "catalogBulkManifest" local PAGE_SIZE = 100 @@ -411,6 +413,17 @@ function CatalogBulkDownload.install(Catalog) dialog = ButtonDialog:new{ title = _("Bulk download settings"), buttons = { + { + { + text = T(_("Folder: %1"), BD.dirpath(shortPath(self:getCurrentDownloadDir(), FOLDER_LABEL_LENGTH))), + callback = function() + UIManager:close(dialog) + self:chooseDownloadDir(function() + self:showBulkDownloadSettings() + end) + end, + }, + }, { { text = T(_("Format: %1"), preset.label), diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_dashboard.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_dashboard.lua index 3099dea6a..85823b9cc 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_dashboard.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_dashboard.lua @@ -350,43 +350,60 @@ function CatalogDashboard.dashboardBooks(dashboard) return books end -function CatalogDashboard:dashboardActionEntries(total_books) +-- Badges come from the server alongside the rest of the dashboard, so a server +-- too old to send them just leaves the tiles unbadged. Not on device is the one +-- tile that never carries a count: learning it would cost the whole-library walk +-- the tile exists to avoid. +function CatalogDashboard:dashboardActionEntries(total_books, browse_counts) + local counts = type(browse_counts) == "table" and browse_counts or {} + local badge = CatalogUtil.formatCount + return { { text = _("In progress"), icon = "dogear.reading", icon_file = CatalogWidgets.assetIconFile("in_progress"), + mandatory = badge(counts.inProgress), kind = "books", params = { sort = "recently_read", readStatus = "reading" }, }, { text = _("On device"), icon = "appbar.filebrowser", - mandatory = tostring(self:onDeviceCount()), + mandatory = badge(self:onDeviceCount()), kind = "on-device", }, + { + text = _("Not on device"), + icon = "appbar.filebrowser", + icon_file = CatalogWidgets.assetIconFile("download"), + kind = "not-on-device", + }, { text = _("Libraries"), icon = "column.two", + mandatory = badge(counts.libraries), kind = "section", section = "libraries", }, { text = _("All Books"), icon = "appbar.pageview", - mandatory = total_books and tostring(total_books) or nil, + mandatory = badge(total_books), kind = "books", params = { sort = "title" }, }, { text = _("Authors"), icon = "bookmark", + mandatory = badge(counts.authors), kind = "section", section = "authors", }, { text = _("Series"), icon = "book.opened", + mandatory = badge(counts.series), kind = "section", section = "series", }, @@ -394,20 +411,17 @@ function CatalogDashboard:dashboardActionEntries(total_books) text = _("Collections"), icon = "texture-box", icon_file = CatalogWidgets.assetIconFile("collections"), + mandatory = badge(counts.collections), kind = "section", section = "collections", }, { text = _("SmartScopes"), icon = "cre.render.working", + mandatory = badge(counts.smartScopes), kind = "section", section = "smart-scopes", }, - { - text = _("Search"), - icon = "appbar.search", - kind = "dashboard-search", - }, } end @@ -933,7 +947,7 @@ function CatalogDashboard:updateDashboardItems(select_number, no_recalculate_dim local context = self.current_context or {} local dashboard = context.dashboard or {} local continue_books = dashboard.continueReading or {} - local action_entries = self:dashboardActionEntries(tonumber(dashboard.totalBooks or dashboard.bookCount)) + local action_entries = self:dashboardActionEntries(tonumber(dashboard.totalBooks or dashboard.bookCount), dashboard.browseCounts) local summary = self:dashboardStatsSummary() or { today_seconds = 0, week_seconds = 0, streak_days = 0 } local stale_sections = type(context.section_stale) == "table" and context.section_stale or {} local highlight = type(dashboard.highlightOfTheDay) == "table" and dashboard.highlightOfTheDay or nil diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_download.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_download.lua index 19b9ecfbe..32baa0bb1 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_download.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_download.lua @@ -113,6 +113,27 @@ function CatalogDownload.install(Catalog) return G_reader_settings:readSetting("download_dir") or G_reader_settings:readSetting("lastdir") end + -- Single and bulk downloads share one destination, KOReader's own + -- download_dir, so the chooser is shared too rather than each dialog + -- keeping its own idea of where files land. + function Catalog:chooseDownloadDir(on_chosen) + require("ui/downloadmgr"):new{ + onConfirm = function(path) + logger.dbg("BookOrbit: download folder set to", path) + if self._manager and self._manager.ui and self._manager.ui.folder_shortcuts + and self._manager.ui.folder_shortcuts.updateShortcut then + self._manager.ui.folder_shortcuts:updateShortcut("download_dir", path) + end + G_reader_settings:saveSetting("download_dir", path) + if on_chosen then + UIManager:nextTick(function() + on_chosen(path) + end) + end + end, + }:chooseDir(self:getCurrentDownloadDir()) + end + function Catalog:getLocalDownloadPath(filename, filetype, device_path, filename_override) local download_dir = self:getCurrentDownloadDir() local relative = resolveRelativeDownloadPath(download_dir, filename, filetype, device_path, filename_override) @@ -200,19 +221,9 @@ function CatalogDownload.install(Catalog) text = _("Choose folder"), callback = function() UIManager:close(dialog) - require("ui/downloadmgr"):new{ - onConfirm = function(path) - logger.dbg("BookOrbit: download folder set to", path) - if self._manager and self._manager.ui and self._manager.ui.folder_shortcuts - and self._manager.ui.folder_shortcuts.updateShortcut then - self._manager.ui.folder_shortcuts:updateShortcut("download_dir", path) - end - G_reader_settings:saveSetting("download_dir", path) - UIManager:nextTick(function() - self:showDownloadDialog(detail, file, filename_overridden and filename or nil) - end) - end, - }:chooseDir(self:getCurrentDownloadDir()) + self:chooseDownloadDir(function() + self:showDownloadDialog(detail, file, filename_overridden and filename or nil) + end) end, }, { diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_util.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_util.lua index 6bd69c8a1..4698b8894 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_util.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_util.lua @@ -20,7 +20,15 @@ CatalogUtil.DEFAULT_GRID_ROWS = 3 -- batch and the page rebuilds once when it lands. CatalogUtil.THUMBNAIL_BATCH_SIZE = 3 CatalogUtil.MAX_RECENT_SEARCHES = 8 +-- The server caps its ids filter at 200, so both device-derived lists are +-- bounded by it rather than by the size of the library. CatalogUtil.ON_DEVICE_MAX_IDS = 200 +CatalogUtil.NOT_ON_DEVICE_MAX_IDS = 200 +-- Not-on-device has no server-side filter, so the list is assembled by walking +-- the newest books. Both bounds keep that walk to a fixed number of requests +-- however large the library is. +CatalogUtil.NOT_ON_DEVICE_SCAN_PAGES = 5 +CatalogUtil.NOT_ON_DEVICE_SCAN_SIZE = 100 CatalogUtil.SORTS = { { id = "title", text = _("Title") }, @@ -120,6 +128,32 @@ function CatalogUtil.formatBytes(bytes) return tostring(bytes) .. " B" end +-- Browse badges sit in a narrow column beside the label, so a five-digit +-- library would eat the room the label needs. Counts stay at most four +-- characters. The scaled forms truncate rather than round, so a badge never +-- claims more than the library holds and 999999 cannot carry to "1000K". +function CatalogUtil.formatCount(value) + local count = tonumber(value) + if not count or count < 0 then return nil end + count = math.floor(count) + if count < 1000 then return tostring(count) end + + local function scaled(divisor, suffix) + local units = count / divisor + if units >= 10 then + return string.format("%d%s", math.floor(units), suffix) + end + local tenths = math.floor(units * 10) / 10 + if tenths == math.floor(tenths) then + return string.format("%d%s", tenths, suffix) + end + return string.format("%.1f%s", tenths, suffix) + end + + if count < 1000000 then return scaled(1000, "K") end + return scaled(1000000, "M") +end + function CatalogUtil.formatDuration(seconds) if not seconds then return nil end local minutes = math.floor(seconds / 60 + 0.5) @@ -213,6 +247,35 @@ function CatalogUtil.shortText(text, max_len) return util.fixUtf8(text:sub(1, max_len - 3), "?") .. "..." end +-- Walks the newest books through fetchPage(page) and keeps the ids isOnDevice +-- rejects. There is no server-side filter for this: which books are linked is +-- local KOReader state the server never sees, so the walk is bounded by both +-- the page count and the server's cap on the ids filter to stay predictable on +-- a large library. Returns nil plus the fetch error if a page fails. +function CatalogUtil.scanNotOnDeviceIds(fetchPage, isOnDevice) + local ids = {} + for page = 1, CatalogUtil.NOT_ON_DEVICE_SCAN_PAGES do + local body, err = fetchPage(page) + if not body then return nil, err end + for _, book in ipairs(body.items or {}) do + if book.id ~= nil and not isOnDevice(book) then + table.insert(ids, book.id) + if #ids >= CatalogUtil.NOT_ON_DEVICE_MAX_IDS then return ids end + end + end + if body.hasNext ~= true then break end + end + return ids +end + +-- Paths carry their meaning at the end, so an over-long one keeps its tail +-- rather than its root the way shortText would. +function CatalogUtil.shortPath(path, max_len) + path = tostring(path or "") + if #path <= max_len then return path end + return "..." .. util.fixUtf8(path:sub(#path - max_len + 4), "?") +end + function CatalogUtil.joinNames(items, key) local names = {} for _, item in ipairs(items or {}) do diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_widgets.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_widgets.lua index c820dce03..57a9cfe69 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_widgets.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_widgets.lua @@ -1414,9 +1414,11 @@ function DashboardBrowseRow:init() fgcolor = Blitbuffer.COLOR_DARK_GRAY, } end - -- Wide counts (a four-digit library) get the width they measure at instead - -- of being clipped to the nominal box. - local count_w = count_widget and math.max(count_box_w, count_widget:getSize().w + Size.padding.small) or 0 + -- Counts are abbreviated to at most four characters upstream, so the column + -- only has to be wide enough for the widest of those rather than for an + -- arbitrary library total. + local count_pad = Size.padding.small + local count_w = count_widget and math.max(count_box_w, count_widget:getSize().w + count_pad) or 0 local label_w = math.max(1, self.dimen.w - icon_box_w - gap - count_w) local left = HorizontalGroup:new{ align = "center" } @@ -1443,10 +1445,13 @@ function DashboardBrowseRow:init() dimen = row_dimen:copy(), LeftContainer:new{ dimen = row_dimen:copy(), left }, } + -- Right-aligned rather than centred in the count box: centring leaves each + -- badge a different distance from the edge, so a column of counts reads as + -- ragged even though every box ends in the same place. if count_widget then table.insert(row, RightContainer:new{ - dimen = row_dimen:copy(), - CenterContainer:new{ dimen = Geom:new{ w = count_w, h = inner_h }, count_widget }, + dimen = Geom:new{ w = math.max(1, row_dimen.w - count_pad), h = inner_h }, + count_widget, }) end diff --git a/koreader-plugin/spec/bookorbit_dashboard_browse_entries_test.lua b/koreader-plugin/spec/bookorbit_dashboard_browse_entries_test.lua new file mode 100644 index 000000000..5930de96e --- /dev/null +++ b/koreader-plugin/spec/bookorbit_dashboard_browse_entries_test.lua @@ -0,0 +1,218 @@ +-- The Browse block is a fixed three-by-three grid, so its entry list has to +-- stay exactly nine long: a tenth destination would be built and then fall off +-- the bottom of the block unseen. Search is not among them because the title +-- bar already carries a search button on every non-detail page, the same reason +-- the non-dashboard root list has never offered it as a row. + +package.loaded["document/documentregistry"] = { hasProvider = function() return true end } +package.loaded["util"] = { + fixUtf8 = function(value) return value end, + urlEncode = function(value) return value end, +} +package.loaded["ffi/util"] = { template = function(value) return value end } +package.loaded["gettext"] = function(text) return text end + +local function widgetClass(height) + return { + new = function(_, opts) + opts = opts or {} + if not opts.getSize then + function opts:getSize() + return { w = 100, h = self.width or height or 20 } + end + end + return opts + end, + } +end + +package.loaded["ffi/blitbuffer"] = { COLOR_DARK_GRAY = 1, COLOR_LIGHT_GRAY = 2 } +package.loaded["ui/widget/button"] = widgetClass() +package.loaded["ui/widget/container/centercontainer"] = widgetClass() +package.loaded["ui/font"] = { getFace = function() return {} end } +package.loaded["ui/geometry"] = widgetClass() +package.loaded["ui/widget/horizontalgroup"] = widgetClass(0) +package.loaded["ui/widget/horizontalspan"] = widgetClass(0) +package.loaded["ui/widget/infomessage"] = widgetClass() +package.loaded["ui/widget/linewidget"] = widgetClass() +package.loaded["ui/network/manager"] = { isConnected = function() return true end } +package.loaded["device"] = { screen = { scaleBySize = function(_, value) return value end } } +package.loaded["ui/size"] = { + line = { thin = 1 }, + padding = { large = 8 }, + span = { horizontal_default = 4, vertical_default = 4 }, +} +package.loaded["ui/widget/textboxwidget"] = widgetClass() +package.loaded["ui/widget/textwidget"] = widgetClass() +package.loaded["ui/uimanager"] = { show = function() end } +package.loaded["ui/widget/verticalgroup"] = widgetClass(0) +package.loaded["ui/widget/verticalspan"] = widgetClass() +package.loaded["bookorbit_stats_reader"] = { + getReadingSummary = function() + return { today_seconds = 0, week_seconds = 0, streak_days = 0 } + end, +} +package.loaded["bookorbit_catalog_widgets"] = { + DashboardCoverCard = widgetClass(), + DashboardHeroCard = widgetClass(), + DashboardHighlightCard = widgetClass(), + DashboardBrowseRow = widgetClass(), + DashboardIconButton = widgetClass(), + assetIconFile = function(name) return "/assets/" .. name .. ".svg" end, + dashboardSectionHeaderRowHeight = function() return 14 end, + buildDashboardSectionHeader = function() + return { getSize = function() return { w = 100, h = 20 } end } + end, + buildStatusLabel = function() + return { getSize = function() return { w = 100, h = 16 } end } + end, + dashboardHighlightCardHeight = function() return 64 end, + coverCardHeight = function(card_w) return card_w + 20 end, + dashboardHeroHeightForCoverCard = function(card_w) return card_w + 24 end, +} + +package.path = "koreader-plugin/bookorbit.koplugin/?.lua;" .. package.path + +local CatalogDashboard = require("bookorbit_catalog_dashboard") + +local function assertEqual(actual, expected, label) + if actual ~= expected then + error(string.format("%s: expected %s, got %s", label, tostring(expected), tostring(actual))) + end +end + +local catalog = {} +for name, fn in pairs(CatalogDashboard) do + if name ~= "install" then catalog[name] = fn end +end +function catalog:onDeviceCount() return 7 end + +local BROWSE_COUNTS = { + inProgress = 4, + libraries = 3, + authors = 812, + series = 96, + collections = 5, + smartScopes = 2, +} + +local entries = catalog:dashboardActionEntries(120, BROWSE_COUNTS) + +-- Three columns by three rows. The block renders what fits and no more. +assertEqual(#entries, 9, "the Browse block holds exactly nine destinations") + +local by_text = {} +local by_kind = {} +for index, entry in ipairs(entries) do + assertEqual(type(entry.text), "string", "entry " .. index .. " is labelled") + assertEqual(entry.text ~= "", true, "entry " .. index .. " has a non-empty label") + assertEqual(entry.icon ~= nil, true, "entry " .. index .. " has a fallback glyph") + assertEqual(by_text[entry.text], nil, "entry " .. index .. " is not a duplicate label") + by_text[entry.text] = entry + by_kind[entry.kind] = (by_kind[entry.kind] or 0) + 1 +end + +-- The title bar's search button reaches the same prompt with the same empty +-- scope, so a row for it only spent a grid cell. +assertEqual(by_text["Search"], nil, "Search is not duplicated into the grid") +assertEqual(by_kind["dashboard-search"], nil, "no entry routes to the search prompt") + +-- The pair the download workflow is built around: what is here, and what is not. +assertEqual(by_text["On device"] ~= nil, true, "On device is offered") +assertEqual(by_text["On device"].kind, "on-device", "On device routes to the linked list") +assertEqual(by_text["On device"].mandatory, "7", "On device carries its count") + +assertEqual(by_text["Not on device"] ~= nil, true, "Not on device is offered") +assertEqual(by_text["Not on device"].kind, "not-on-device", "Not on device routes to the walk") +-- The count would cost a full library walk to learn, so the row stays quiet +-- rather than showing a number that is only a guess. +assertEqual(by_text["Not on device"].mandatory, nil, "Not on device shows no count") +assertEqual( + by_text["Not on device"].icon_file ~= by_text["On device"].icon_file, + true, + "the two device rows are told apart by their icons" +) + +-- The catalogs reached through the grid still are. +for _, label in ipairs({ "In progress", "Libraries", "All Books", "Authors", "Series", "Collections", "SmartScopes" }) do + assertEqual(by_text[label] ~= nil, true, label .. " survived the swap") +end +assertEqual(by_text["All Books"].mandatory, "120", "All Books carries the library total") + +-- Every tile the server can count carries its badge. Eight of the nine: the +-- device pair is local, All Books rides the dashboard total, and the rest come +-- from the browse counts. +assertEqual(by_text["In progress"].mandatory, "4", "In progress badge") +assertEqual(by_text["Libraries"].mandatory, "3", "Libraries badge") +assertEqual(by_text["Authors"].mandatory, "812", "Authors badge") +assertEqual(by_text["Series"].mandatory, "96", "Series badge") +assertEqual(by_text["Collections"].mandatory, "5", "Collections badge") +assertEqual(by_text["SmartScopes"].mandatory, "2", "SmartScopes badge") + +local badged = 0 +for _, entry in ipairs(entries) do + if entry.mandatory ~= nil then badged = badged + 1 end +end +assertEqual(badged, 8, "eight of the nine tiles can be counted") + +-- Zero is a real answer and has to render, not be swallowed as falsy the way a +-- plain truthiness check on the count would. +local empty = catalog:dashboardActionEntries(0, { + inProgress = 0, libraries = 0, authors = 0, series = 0, collections = 0, smartScopes = 0, +}) +for _, entry in ipairs(empty) do + if entry.kind ~= "not-on-device" then + assertEqual(entry.mandatory ~= nil, true, entry.text .. " renders a zero count") + end +end + +-- A server too old to send the counts, and a server that sends nonsense, both +-- degrade to unbadged tiles rather than rendering "nil" into the grid. +for _, payload in ipairs({ { nil }, { "not a table" }, { { authors = "many", series = {} } } }) do + local degraded = catalog:dashboardActionEntries(nil, payload[1]) + assertEqual(#degraded, 9, "the grid is still full") + for _, entry in ipairs(degraded) do + if entry.kind ~= "on-device" then + assertEqual(entry.mandatory, nil, entry.text .. " has no badge without usable counts") + end + end +end + +-- Badges share a narrow column with the label, so the string is bounded at four +-- characters however large the library gets. +local CatalogUtil = require("bookorbit_catalog_util") +local formatCount = CatalogUtil.formatCount + +assertEqual(formatCount(0), "0", "zero is a count, not an absence") +assertEqual(formatCount(7), "7", "single digit") +assertEqual(formatCount(999), "999", "the last unscaled value") +assertEqual(formatCount(1000), "1K", "a whole thousand drops its decimal") +assertEqual(formatCount(1500), "1.5K", "a half thousand keeps one decimal") +assertEqual(formatCount(9999), "9.9K", "just under ten thousand") +assertEqual(formatCount(10000), "10K", "ten thousand and up lose the decimal") +assertEqual(formatCount(12431), "12K", "a large library") +assertEqual(formatCount(999999), "999K", "the last value before millions") +assertEqual(formatCount(1000000), "1M", "millions scale again") +assertEqual(formatCount(1500000), "1.5M", "millions keep one decimal") + +-- Truncating, not rounding: a badge must never claim more books than are there, +-- and rounding 999999 up would carry it to a five-character "1000K". +assertEqual(formatCount(1999), "1.9K", "the scaled value truncates") +assertEqual(formatCount(1099), "1K", "truncation can drop the decimal entirely") + +for _, value in ipairs({ 0, 7, 999, 1000, 1500, 9999, 10000, 999999, 1000000, 1500000 }) do + assertEqual(#formatCount(value) <= 4, true, "the badge for " .. value .. " fits the column") +end + +-- Anything that is not a usable count leaves the tile unbadged rather than +-- rendering a placeholder into the grid. +for _, value in ipairs({ "many", -1 }) do + assertEqual(formatCount(value), nil, "unusable count " .. tostring(value) .. " has no badge") +end +assertEqual(formatCount(nil), nil, "a missing count has no badge") +assertEqual(formatCount({}), nil, "a malformed count has no badge") + +-- A count arriving as a numeric string still counts, since the payload is JSON. +assertEqual(formatCount("1500"), "1.5K", "a numeric string is accepted") + +print("bookorbit_dashboard_browse_entries_test.lua: ok") diff --git a/koreader-plugin/spec/bookorbit_download_folder_test.lua b/koreader-plugin/spec/bookorbit_download_folder_test.lua new file mode 100644 index 000000000..674a788fa --- /dev/null +++ b/koreader-plugin/spec/bookorbit_download_folder_test.lua @@ -0,0 +1,163 @@ +-- Single and bulk downloads write to one destination, KOReader's own +-- download_dir, so both dialogs have to offer the same chooser and see each +-- other's choice. The bulk popup previously only reported the folder in its +-- confirmation text, which left the setting reachable from the single-book +-- dialog alone. + +local shown = {} +local next_ticks = {} + +package.loaded["ui/bidi"] = { + dirpath = function(value) return value end, + filepath = function(value) return value end, +} +package.loaded["datastorage"] = { + getSettingsDir = function() return "/tmp/bookorbit-spec" end, + getDataDir = function() return "/tmp/bookorbit-spec" end, +} +package.loaded["dump"] = function(value) return tostring(value) end +package.loaded["document/documentregistry"] = { hasProvider = function() return true end } +package.loaded["ui/widget/buttondialog"] = { + new = function(_, opts) + opts = opts or {} + function opts:setTitle(title) self.title = title end + return opts + end, +} +package.loaded["ui/widget/infomessage"] = { new = function(_, opts) return opts or {} end } +package.loaded["ui/widget/confirmbox"] = package.loaded["ui/widget/infomessage"] +package.loaded["ui/widget/inputdialog"] = package.loaded["ui/widget/infomessage"] +package.loaded["ui/widget/notification"] = package.loaded["ui/widget/infomessage"] +package.loaded["ui/uimanager"] = { + show = function(_, widget) table.insert(shown, widget) end, + close = function() end, + nextTick = function(_, callback) table.insert(next_ticks, callback) end, + scheduleIn = function(_, _, callback) table.insert(next_ticks, callback) end, + preventStandby = function() end, + allowStandby = function() end, + forceRePaint = function() end, +} +package.loaded["libs/libkoreader-lfs"] = { + attributes = function() return nil end, + dir = function() return function() return nil end end, +} +package.loaded["logger"] = { dbg = function() end, warn = function() end } +package.loaded["util"] = { + fixUtf8 = function(value) return value end, + getSafeFilename = function(value) return value end, + makePath = function() end, + trim = function(value) return value end, +} +package.loaded["ffi/util"] = { + template = function(value, ...) + local values = { ... } + return (tostring(value):gsub("%%(%d+)", function(index) + return tostring(values[tonumber(index)]) + end)) + end, +} +package.loaded["gettext"] = function(text) return text end +package.loaded["bookorbit_state_manager"] = {} +package.loaded["bookorbit_download_transfer"] = {} +package.loaded["bookorbit_bulk_checkpoint"] = { MAX_FAILURES = 8 } +package.loaded["bookorbit_capabilities"] = { has = function() return false end } + +-- The chooser is KOReader's own directory browser; the test drives its confirm +-- callback directly rather than rendering it. +local chooser = { opened = 0, start_dir = nil, confirm = nil } +package.loaded["ui/downloadmgr"] = { + new = function(_, opts) + return { + chooseDir = function(_, start_dir) + chooser.opened = chooser.opened + 1 + chooser.start_dir = start_dir + chooser.confirm = opts.onConfirm + end, + } + end, +} + +package.path = "koreader-plugin/bookorbit.koplugin/?.lua;" .. package.path + +local BulkDownload = require("bookorbit_catalog_bulk_download") +local CatalogDownload = require("bookorbit_catalog_download") + +local function assertEqual(actual, expected, label) + if actual ~= expected then + error(string.format("%s: expected %s, got %s", label, tostring(expected), tostring(actual))) + end +end + +local stored = { download_dir = "/mnt/onboard/Books" } +local saved = {} +G_reader_settings = { + readSetting = function(_, key) return stored[key] end, + saveSetting = function(_, key, value) + stored[key] = value + table.insert(saved, { key = key, value = value }) + end, + flush = function() end, +} + +local Catalog = {} +BulkDownload.install(Catalog) +CatalogDownload.install(Catalog) +Catalog.settings = {} +Catalog:initBulkDownloadState() + +local function findButton(dialog, prefix) + for _, row in ipairs(dialog.buttons or {}) do + for _, button in ipairs(row) do + if button.text and button.text:sub(1, #prefix) == prefix then return button end + end + end + return nil +end + +-- The bulk settings popup offers the folder alongside the other two knobs. +Catalog:showBulkDownloadSettings() +local settings_dialog = shown[#shown] +assertEqual(settings_dialog.title, "Bulk download settings", "the settings popup opened") + +local folder_button = findButton(settings_dialog, "Folder:") +assertEqual(folder_button ~= nil, true, "the folder is offered as a button") +assertEqual(folder_button.text, "Folder: /mnt/onboard/Books", "the button reports the current folder") +assertEqual(findButton(settings_dialog, "Format:") ~= nil, true, "the format knob is still offered") +assertEqual(findButton(settings_dialog, "Existing files:") ~= nil, true, "the existing-files knob is still offered") + +-- Choosing a folder writes the one setting the download path is built from. +folder_button.callback() +assertEqual(chooser.opened, 1, "the chooser opened") +assertEqual(chooser.start_dir, "/mnt/onboard/Books", "the chooser starts at the current folder") + +chooser.confirm("/mnt/onboard/Books/BookOrbit") +assertEqual(#saved, 1, "one setting was written") +assertEqual(saved[1].key, "download_dir", "the shared download_dir is what changed") +assertEqual(saved[1].value, "/mnt/onboard/Books/BookOrbit", "the chosen path was stored") + +-- The popup reopens on the next tick so the new folder is visible without the +-- user having to find their way back into the menu. +assertEqual(#next_ticks, 1, "the caller was scheduled to reopen") +next_ticks[1]() +assertEqual(findButton(shown[#shown], "Folder:").text, "Folder: /mnt/onboard/Books/BookOrbit", + "the reopened popup shows the new folder") + +-- The same choice is what a bulk run then downloads into, and what its +-- confirmation reports. +assertEqual(Catalog:getCurrentDownloadDir(), "/mnt/onboard/Books/BookOrbit", "bulk downloads follow the choice") +local title = Catalog:bulkConfirmationTitle({ total = 3, label = "Current page" }) +assertEqual(title:find("Folder: /mnt/onboard/Books/BookOrbit", 1, true) ~= nil, true, + "the confirmation reports the chosen folder") + +-- An over-long path keeps the end, which is the part that says where the files +-- actually land; truncating the other way would leave every folder looking the +-- same. +local CatalogUtil = require("bookorbit_catalog_util") +local deep = "/mnt/onboard/Books/Fiction/Science Fiction/Series/Long Name" +local short = CatalogUtil.shortPath(deep, 24) +assertEqual(#short, 24, "the label is bounded") +assertEqual(short:sub(1, 3), "...", "the truncation is marked") +assertEqual(deep:sub(-10), short:sub(-10), "the tail of the path survives") +assertEqual(CatalogUtil.shortPath("/short", 24), "/short", "a short path is left alone") + +print("bookorbit_download_folder_test.lua: ok") diff --git a/koreader-plugin/spec/bookorbit_not_on_device_test.lua b/koreader-plugin/spec/bookorbit_not_on_device_test.lua new file mode 100644 index 000000000..c78659c48 --- /dev/null +++ b/koreader-plugin/spec/bookorbit_not_on_device_test.lua @@ -0,0 +1,140 @@ +-- "Not on device" is the one book source with no server-side filter behind it: +-- which books are linked is local KOReader state the server never sees. The +-- list is therefore walked client-side, and what matters is that the walk stays +-- bounded on a large library, stops as soon as the server runs out of pages, +-- and reports a failed page instead of silently returning a short list. + +package.loaded["document/documentregistry"] = { + hasProvider = function() + return true + end, +} + +package.loaded["util"] = { + fixUtf8 = function(value) + return value + end, +} + +package.loaded["ffi/util"] = { + template = function(value) + return value + end, +} + +package.loaded["gettext"] = function(text) + return text +end + +package.path = "koreader-plugin/bookorbit.koplugin/?.lua;" .. package.path + +local CatalogUtil = require("bookorbit_catalog_util") + +local function assertEqual(actual, expected, label) + if actual ~= expected then + error(string.format("%s: expected %s, got %s", label, tostring(expected), tostring(actual))) + end +end + +local function books(from, to) + local items = {} + for id = from, to do + table.insert(items, { id = id }) + end + return items +end + +local function pager(pages) + local requested = {} + return function(page) + table.insert(requested, page) + local body = pages[page] + if not body then return nil, 500 end + return body + end, requested +end + +local function keepAll() return false end + +-- The books already here are dropped, and the rest keep the server's order. +local fetch, requested = pager({ + { items = { { id = 1 }, { id = 2 }, { id = 3 }, { id = 4 } }, hasNext = false }, +}) +local on_device = { [2] = true, [4] = true } +local ids = CatalogUtil.scanNotOnDeviceIds(fetch, function(book) + return on_device[book.id] == true +end) +assertEqual(#ids, 2, "linked books are dropped") +assertEqual(ids[1], 1, "first kept id") +assertEqual(ids[2], 3, "order follows the server") +assertEqual(#requested, 1, "a single page was enough") + +-- A book with no id cannot be asked for again, so it never reaches the list. +local unidentified = CatalogUtil.scanNotOnDeviceIds(function() + return { items = { { id = 1 }, {}, { id = 3 } }, hasNext = false } +end, keepAll) +assertEqual(#unidentified, 2, "an entry without an id is skipped") + +-- hasNext false ends the walk even though the page budget is not spent. +local short_fetch, short_requested = pager({ + { items = books(1, 10), hasNext = true }, + { items = books(11, 15), hasNext = false }, + { items = books(16, 20), hasNext = true }, +}) +local short = CatalogUtil.scanNotOnDeviceIds(short_fetch, keepAll) +assertEqual(#short, 15, "the walk stops when the server says there is no next page") +assertEqual(#short_requested, 2, "no page is requested past the end") + +-- A library larger than the walk never turns into an unbounded scan: the page +-- budget caps the requests whatever the server keeps offering. +local endless_pages = {} +for page = 1, CatalogUtil.NOT_ON_DEVICE_SCAN_PAGES + 3 do + endless_pages[page] = { items = books((page - 1) * 10 + 1, page * 10), hasNext = true } +end +local endless_fetch, endless_requested = pager(endless_pages) +local bounded = CatalogUtil.scanNotOnDeviceIds(endless_fetch, keepAll) +assertEqual(#endless_requested, CatalogUtil.NOT_ON_DEVICE_SCAN_PAGES, "the page budget bounds the walk") +assertEqual(#bounded, CatalogUtil.NOT_ON_DEVICE_SCAN_PAGES * 10, "every scanned book is kept") + +-- The server caps its ids filter, so the walk stops collecting at that cap +-- rather than building a list the follow-up request would reject. +local cap_pages = {} +for page = 1, CatalogUtil.NOT_ON_DEVICE_SCAN_PAGES do + local from = (page - 1) * CatalogUtil.NOT_ON_DEVICE_SCAN_SIZE + 1 + cap_pages[page] = { + items = books(from, from + CatalogUtil.NOT_ON_DEVICE_SCAN_SIZE - 1), + hasNext = true, + } +end +local cap_fetch, cap_requested = pager(cap_pages) +local capped = CatalogUtil.scanNotOnDeviceIds(cap_fetch, keepAll) +assertEqual(#capped, CatalogUtil.NOT_ON_DEVICE_MAX_IDS, "collection stops at the ids cap") +assertEqual( + #cap_requested <= CatalogUtil.NOT_ON_DEVICE_SCAN_PAGES, + true, + "the cap is reached without exceeding the page budget" +) + +-- A failed page is an error, not a short list: returning what was gathered so +-- far would read as "these are the books you are missing". +local failed, err = CatalogUtil.scanNotOnDeviceIds(function(page) + if page == 1 then return { items = books(1, 10), hasNext = true } end + return nil, 503 +end, keepAll) +assertEqual(failed, nil, "a failed page returns no list") +assertEqual(err, 503, "the fetch error is passed through") + +-- Cancelling mid-walk has to stay distinguishable so the caller can stay quiet +-- rather than offering a retry the user did not ask for. +local _, cancelled = CatalogUtil.scanNotOnDeviceIds(function() + return nil, "cancelled" +end, keepAll) +assertEqual(cancelled, "cancelled", "cancellation is reported as itself") + +-- Everything already here is the empty case the caller messages separately. +local none = CatalogUtil.scanNotOnDeviceIds(function() + return { items = books(1, 5), hasNext = false } +end, function() return true end) +assertEqual(#none, 0, "a fully downloaded library yields nothing") + +print("bookorbit_not_on_device_test.lua: ok") diff --git a/packages/types/src/koreader.ts b/packages/types/src/koreader.ts index 7daef2b0c..469b67fab 100644 --- a/packages/types/src/koreader.ts +++ b/packages/types/src/koreader.ts @@ -300,11 +300,25 @@ export interface KoreaderCatalogDashboardSection { books: KoreaderCatalogBookListItem[]; } +// Totals behind the dashboard's Browse tiles. Optional so a plugin reading an +// older server simply renders the tiles without badges. There is deliberately +// no not-on-device total: which books are on a device is local KOReader state +// the server never sees. +export interface KoreaderCatalogBrowseCounts { + inProgress: number; + libraries: number; + authors: number; + series: number; + collections: number; + smartScopes: number; +} + export interface KoreaderCatalogDashboardResponse { generatedAt: string; username: string; displayName: string; totalBooks: number; + browseCounts?: KoreaderCatalogBrowseCounts; sections: KoreaderCatalogEntry[]; continueReading: KoreaderCatalogBookListItem[]; // Legacy Discover row. Populated only for requests that name no section, so a diff --git a/server/src/modules/browse-counts/browse-counts.module.ts b/server/src/modules/browse-counts/browse-counts.module.ts index 69a6aa8d9..9311b06cc 100644 --- a/server/src/modules/browse-counts/browse-counts.module.ts +++ b/server/src/modules/browse-counts/browse-counts.module.ts @@ -10,5 +10,6 @@ import { BrowseCountsService } from './browse-counts.service'; imports: [AuthorsModule, SeriesModule, AnnotationModule], controllers: [BrowseCountsController], providers: [BrowseCountsService], + exports: [BrowseCountsService], }) export class BrowseCountsModule {} diff --git a/server/src/modules/koreader/koreader-catalog.service.test.ts b/server/src/modules/koreader/koreader-catalog.service.test.ts index c66842b7d..43a5ca4ac 100644 --- a/server/src/modules/koreader/koreader-catalog.service.test.ts +++ b/server/src/modules/koreader/koreader-catalog.service.test.ts @@ -100,6 +100,10 @@ function makeService( getDistinctAuthorsPage: vi.fn().mockResolvedValue({ items: [{ name: 'Frank Herbert', bookCount: 2 }], hasNext: false }), getDistinctSeriesPage: vi.fn().mockResolvedValue({ items: [{ id: 42, name: 'Dune', bookCount: 6 }], hasNext: false }), getBooksPage: vi.fn().mockResolvedValue({ entries: [], total: 0 }), + countBooks: vi.fn().mockResolvedValue(99), + countUserCollections: vi.fn().mockResolvedValue(1), + countUserSmartScopes: vi.fn().mockResolvedValue(1), + getAccessibleLibraryIds: vi.fn().mockResolvedValue([1]), getBookManifestPage: vi.fn().mockResolvedValue({ rows: [], hasNext: false }), getRandomBooks: vi.fn().mockResolvedValue([]), }; @@ -215,6 +219,10 @@ function makeService( getLibraryVersion: vi.fn().mockResolvedValue('lib-v1'), }; + const browseCountsService = { + getCounts: vi.fn().mockResolvedValue({ authors: 812, series: 96, annotations: 40 }), + }; + const service = new KoreaderCatalogService( opdsBookService as never, bookService as never, @@ -222,6 +230,7 @@ function makeService( userBookStatusService as never, dashboardService as never, dashboardWidgetService as never, + browseCountsService as never, recommendationService as never, { isCrossPlatformPathSanitizationEnabled: vi.fn().mockResolvedValue(true) } as never, { @@ -240,6 +249,7 @@ function makeService( userBookStatusService, dashboardService, dashboardWidgetService, + browseCountsService, recommendationService, pluginService, }; @@ -329,7 +339,7 @@ describe('KoreaderCatalogService', () => { const dashboard = await service.getDashboard(user); expect(opdsBookService.getBooksPage).toHaveBeenCalledWith(7, 'recently_read', 1, 5, { readStatus: 'reading' }, false, user.contentFilters); - expect(opdsBookService.getBooksPage).toHaveBeenCalledWith(7, 'title_asc', 1, 1, {}, false, user.contentFilters); + expect(opdsBookService.countBooks).toHaveBeenCalledWith(7, {}, false, user.contentFilters); expect(opdsBookService.getRandomBooks).toHaveBeenCalledWith(7, 12, false, user.contentFilters); expect(dashboard.sections.map((section) => section.id)).toContain('all-books'); expect(dashboard.username).toBe('testuser'); @@ -346,6 +356,59 @@ describe('KoreaderCatalogService', () => { expect(dashboardWidgetService.getHighlightOfTheDay).toHaveBeenCalledWith(user); }); + it('carries browse counts for every dashboard tile the server can count', async () => { + const { service, opdsBookService, browseCountsService } = makeService(); + const user = makeUser({ id: 7 }); + opdsBookService.countBooks.mockResolvedValue(12); + opdsBookService.getAccessibleLibraryIds.mockResolvedValue([1, 2]); + opdsBookService.countUserCollections.mockResolvedValue(1); + opdsBookService.countUserSmartScopes.mockResolvedValue(3); + + const dashboard = await service.getDashboard(user); + + expect(dashboard.browseCounts).toEqual({ + inProgress: 12, + libraries: 2, + authors: 812, + series: 96, + collections: 1, + smartScopes: 3, + }); + // Authors and series come from the cached sidebar totals rather than being + // recomputed per dashboard load. + expect(browseCountsService.getCounts).toHaveBeenCalledWith(user); + expect(opdsBookService.countBooks).toHaveBeenCalledWith(7, { readStatus: 'reading' }, false, user.contentFilters); + }); + + it('counts entities the requesting user can reach, not every row in the table', async () => { + const { service, opdsBookService } = makeService(); + const user = makeUser({ id: 7 }); + opdsBookService.countBooks.mockResolvedValue(0); + opdsBookService.getAccessibleLibraryIds.mockResolvedValue([]); + opdsBookService.countUserCollections.mockResolvedValue(0); + opdsBookService.countUserSmartScopes.mockResolvedValue(0); + + const dashboard = await service.getDashboard(user); + + expect(opdsBookService.getAccessibleLibraryIds).toHaveBeenCalledWith(7, false); + expect(opdsBookService.countUserCollections).toHaveBeenCalledWith(7); + expect(opdsBookService.countUserSmartScopes).toHaveBeenCalledWith(7); + // The badge never reads a per-entity book count, so the list methods that + // aggregate them must stay out of the dashboard path entirely. + expect(opdsBookService.getAccessibleLibraries).not.toHaveBeenCalled(); + expect(opdsBookService.getUserCollections).not.toHaveBeenCalled(); + // Zero has to survive to the wire: the plugin renders it, and an absent + // field is how it detects a server that cannot count at all. + expect(dashboard.browseCounts).toEqual({ + inProgress: 0, + libraries: 0, + authors: 812, + series: 96, + collections: 0, + smartScopes: 0, + }); + }); + function makeEntry(id: number, title: string) { return { id, @@ -517,7 +580,7 @@ describe('KoreaderCatalogService', () => { it('maps section entries to scoped book links and content-filtered counts', async () => { const { service, opdsBookService } = makeService(); - opdsBookService.getBooksPage.mockResolvedValue({ entries: [], total: 3 }); + opdsBookService.countBooks.mockResolvedValue(3); const user = makeUser({ id: 7 }); const libraries = await service.getSectionEntries(user, 'libraries'); @@ -533,7 +596,7 @@ describe('KoreaderCatalogService', () => { booksHref: '/api/v1/koreader/plugin/catalog/books?sort=title&libraryId=1', }), ); - expect(opdsBookService.getBooksPage).toHaveBeenCalledWith(7, 'title_asc', 1, 1, { libraryId: 1 }, false, user.contentFilters); + expect(opdsBookService.countBooks).toHaveBeenCalledWith(7, { libraryId: 1 }, false, user.contentFilters); expect(authors.items[0]!.booksHref).toContain('author=Frank+Herbert'); expect(series.items[0]).toEqual( expect.objectContaining({ @@ -828,27 +891,18 @@ describe('KoreaderCatalogService', () => { it('includes a series read-through summary on series-scoped pages', async () => { const { service, opdsBookService } = makeService(); - opdsBookService.getBooksPage - .mockResolvedValueOnce({ entries: [], total: 6 }) - .mockResolvedValueOnce({ entries: [], total: 6 }) - .mockResolvedValueOnce({ entries: [], total: 2 }); + opdsBookService.getBooksPage.mockResolvedValueOnce({ entries: [], total: 6 }); + opdsBookService.countBooks.mockResolvedValueOnce(6).mockResolvedValueOnce(2); const user = makeUser({ id: 7 }); const result = await service.getBooksPage(user, Object.assign(new KoreaderCatalogBooksQueryDto(), { seriesId: 42, sort: 'series' })); expect(result.seriesSummary).toEqual({ total: 6, finished: 2 }); expect(opdsBookService.getBooksPage).toHaveBeenNthCalledWith(1, 7, 'series_asc', 1, 20, { seriesId: 42 }, false, user.contentFilters); - expect(opdsBookService.getBooksPage).toHaveBeenNthCalledWith(2, 7, 'title_asc', 1, 1, { seriesId: 42 }, false, user.contentFilters); - expect(opdsBookService.getBooksPage).toHaveBeenNthCalledWith( - 3, - 7, - 'title_asc', - 1, - 1, - { seriesId: 42, readStatus: 'finished' }, - false, - user.contentFilters, - ); + // The two summary totals are counted, not paged: neither fetches a row. + expect(opdsBookService.countBooks).toHaveBeenNthCalledWith(1, 7, { seriesId: 42 }, false, user.contentFilters); + expect(opdsBookService.countBooks).toHaveBeenNthCalledWith(2, 7, { seriesId: 42, readStatus: 'finished' }, false, user.contentFilters); + expect(opdsBookService.getBooksPage).toHaveBeenCalledTimes(1); }); it('omits the series summary for non-series listings', async () => { diff --git a/server/src/modules/koreader/koreader-catalog.service.ts b/server/src/modules/koreader/koreader-catalog.service.ts index 59983f5d2..1b1363044 100644 --- a/server/src/modules/koreader/koreader-catalog.service.ts +++ b/server/src/modules/koreader/koreader-catalog.service.ts @@ -11,6 +11,7 @@ import { DEFAULT_KOREADER_DEVICE_PATTERN, resolveUploadPath } from '@bookorbit/t import type { KoreaderCatalogBookDetail, KoreaderCatalogBookListItem, + KoreaderCatalogBrowseCounts, KoreaderCatalogDashboardResponse, KoreaderCatalogDashboardSection, KoreaderCatalogDashboardSectionResponse, @@ -42,6 +43,7 @@ import { contentDispositionHeader } from '../../common/utils/content-disposition import { storageConfig } from '../../config/config'; import { BookReadService } from '../book/book-read.service'; import { BookService } from '../book/book.service'; +import { BrowseCountsService } from '../browse-counts/browse-counts.service'; import type { BookDetailDto } from '../book/dto/book-detail.dto'; import { DashboardService } from '../dashboard/dashboard.service'; import { DashboardWidgetService } from '../dashboard/dashboard-widget.service'; @@ -140,6 +142,7 @@ export class KoreaderCatalogService { private readonly userBookStatusService: UserBookStatusService, private readonly dashboardService: DashboardService, private readonly dashboardWidgetService: DashboardWidgetService, + private readonly browseCountsService: BrowseCountsService, private readonly recommendationService: RecommendationService, private readonly appSettingsService: AppSettingsService, private readonly koreaderService: KoreaderService, @@ -159,7 +162,7 @@ export class KoreaderCatalogService { readStatus: 'reading' as const, }); - const [continueReading, discover, dashboardSection, readingGoal, readingStreak, highlightOfTheDay, totalBooks] = await Promise.all([ + const [continueReading, discover, dashboardSection, readingGoal, readingStreak, highlightOfTheDay, totalBooks, browseCounts] = await Promise.all([ this.getBooksPage(user, continueReadingQuery), section ? Promise.resolve([]) : this.buildDiscover(user), section ? this.buildDashboardSection(user, section) : Promise.resolve(undefined), @@ -167,6 +170,7 @@ export class KoreaderCatalogService { this.dashboardWidgetService.getReadingStreak(user), this.dashboardWidgetService.getHighlightOfTheDay(user), this.countBooks(user, {}), + this.buildBrowseCounts(user), ]); return { @@ -174,6 +178,7 @@ export class KoreaderCatalogService { username: user.username, displayName: user.name || user.username, totalBooks, + browseCounts, sections: ROOT_SECTIONS.map((rootSection) => ({ ...rootSection })), continueReading: continueReading.items, discover, @@ -623,6 +628,29 @@ export class KoreaderCatalogService { return suffix ? `${CATALOG_BASE}/sections/${section}?${suffix}` : `${CATALOG_BASE}/sections/${section}`; } + // Badges for the dashboard Browse tiles. Every total here is a count query: + // authors and series reuse the cached ones the web sidebar already asks for, + // and the rest deliberately avoid the list methods behind the section + // endpoints, which aggregate per-entity book counts a badge never reads. + private async buildBrowseCounts(user: RequestUser): Promise { + const [inProgress, sidebarCounts, libraryIds, collections, smartScopes] = await Promise.all([ + this.countBooks(user, { readStatus: 'reading' }), + this.browseCountsService.getCounts(user), + this.opdsBookService.getAccessibleLibraryIds(user.id, user.isSuperuser), + this.opdsBookService.countUserCollections(user.id), + this.opdsBookService.countUserSmartScopes(user.id), + ]); + + return { + inProgress, + libraries: libraryIds.length, + authors: sidebarCounts.authors, + series: sidebarCounts.series, + collections, + smartScopes, + }; + } + private async countBooks( user: RequestUser, filters: { @@ -633,8 +661,7 @@ export class KoreaderCatalogService { readStatus?: 'unread' | 'reading' | 'finished'; }, ): Promise { - const result = await this.opdsBookService.getBooksPage(user.id, 'title_asc', 1, 1, filters, user.isSuperuser, user.contentFilters); - return result.total; + return this.opdsBookService.countBooks(user.id, filters, user.isSuperuser, user.contentFilters); } private async computeSeriesSummary(user: RequestUser, filters: { series?: string; seriesId?: number }): Promise { diff --git a/server/src/modules/koreader/koreader.module.ts b/server/src/modules/koreader/koreader.module.ts index 5e646bf17..160506630 100644 --- a/server/src/modules/koreader/koreader.module.ts +++ b/server/src/modules/koreader/koreader.module.ts @@ -6,6 +6,7 @@ import { AppSettingsModule } from '../app-settings/app-settings.module'; import { AnnotationModule } from '../annotation/annotation.module'; import { BookmarkModule } from '../bookmark/bookmark.module'; import { BookModule } from '../book/book.module'; +import { BrowseCountsModule } from '../browse-counts/browse-counts.module'; import { DashboardModule } from '../dashboard/dashboard.module'; import { OpdsModule } from '../opds/opds.module'; import { PositionConverterModule } from '../position-converter/position-converter.module'; @@ -43,6 +44,7 @@ import { KoreaderStatsService } from './koreader-stats.service'; AnnotationModule, BookModule, BookmarkModule, + BrowseCountsModule, DashboardModule, OpdsModule, PositionConverterModule, diff --git a/server/src/modules/opds/opds-book.service.ts b/server/src/modules/opds/opds-book.service.ts index 098934175..524e15be2 100644 --- a/server/src/modules/opds/opds-book.service.ts +++ b/server/src/modules/opds/opds-book.service.ts @@ -590,6 +590,31 @@ export class OpdsBookService { .orderBy(collections.name); } + // Badge counts only need the totals, so these skip the per-entity book counts + // their list siblings compute. Counting collections through getUserCollections + // would aggregate over collection_books just to read the row count back. + async countUserCollections(userId: number): Promise { + const [row] = await this.db.select({ total: count() }).from(collections).where(eq(collections.userId, userId)); + return Number(row?.total ?? 0); + } + + async countUserSmartScopes(userId: number): Promise { + const [row] = await this.db + .select({ total: count() }) + .from(smartScopes) + .where(or(eq(smartScopes.userId, userId), eq(smartScopes.isPublic, true))); + return Number(row?.total ?? 0); + } + + // The scope is shared with getBooksPage so a count can never disagree with the + // list it labels, but nothing is fetched or hydrated to produce it. + async countBooks(userId: number, filters?: OpdsBookFilters, isSuperuser = false, contentFilters?: ContentFilterRules): Promise { + const scope = await this.buildCatalogScope(userId, filters, isSuperuser, contentFilters); + if (!scope) return 0; + const [row] = await this.db.select({ total: count() }).from(books).leftJoin(bookMetadata, eq(bookMetadata.bookId, books.id)).where(scope.where); + return Number(row?.total ?? 0); + } + async getUserSmartScopes(userId: number) { return this.db .select({