diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua index 9fff6e355..d59c8973b 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua @@ -307,9 +307,7 @@ function BookOrbitCatalog:init() self.paths = self.stack UIManager:nextTick(function() self:hydrateOnDeviceMaps() - if self:shouldRefreshDashboardOnOpen() then - self:loadDashboardRoot(true, { invisible = true }) - end + self:refreshDashboardOnOpen() -- One-off cache migration, queued behind the refresh so its directory -- walk cannot delay the first dashboard request. UIManager:scheduleIn(1.0, function() @@ -475,10 +473,15 @@ function BookOrbitCatalog:markStackDirty() end end +-- Going home shows the cached dashboard at once and refreshes it behind the +-- redraw, like opening the catalog does, so it never waits on a connection. function BookOrbitCatalog:goDashboard() self.stack = {} - self:updateReturnPath() - self:loadDashboardRoot(true) + local items, context = self:initialDashboardContext() + self:switchTo(context.title or self.title, items, context, false) + UIManager:nextTick(function() + self:refreshDashboardOnOpen() + end) end function BookOrbitCatalog:sectionSubtitle(page, q, has_next) @@ -2421,11 +2424,9 @@ function BookOrbitCatalog:onReturn() if self.current_context.kind == "dashboard" then self:scheduleThumbnailDownloads(self.dashboardBooks(self.current_context.dashboard)) end - if self:shouldRefreshDashboardOnOpen() then - UIManager:nextTick(function() - self:loadDashboardRoot(true, { invisible = true }) - end) - end + UIManager:nextTick(function() + self:refreshDashboardOnOpen() + end) end self:updateLeftIcon() if dirty then diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_dashboard.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_dashboard.lua index 8b048e762..b45b1b60f 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_dashboard.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_dashboard.lua @@ -178,22 +178,22 @@ function CatalogDashboard:cachedDashboardContext(cached, opts) end -- What the catalog widget opens on, built without touching the network: the --- cached dashboard when there is one, otherwise a placeholder the pending --- refresh replaces in place. +-- cached dashboard when there is one, otherwise a placeholder the refresh +-- replaces in place. Offline the placeholder says the dashboard could not be +-- loaded, because a connection may never come. function CatalogDashboard:initialDashboardContext() local cached = self:dashboardCache() if cached then return self:cachedDashboardContext(cached) end - if self:shouldRefreshDashboardOnOpen() then + if NetworkMgr:isConnected() then return self:dashboardContext(nil, { stale = true, loading = true }) end return self:dashboardContext(nil, { stale = true, unavailable = true }) end --- An offline-tolerant open (auto-open, offline browse) never asks for a --- connection on its own; only an explicit browse goes through the connected --- gate. +-- An offline-tolerant catalog never asks for a connection on its own. An +-- explicit open is not offline-tolerant until its first refresh has asked. function CatalogDashboard:shouldRefreshDashboardOnOpen() if self.prefer_cached_dashboard and not NetworkMgr:isConnected() then return false @@ -201,6 +201,17 @@ function CatalogDashboard:shouldRefreshDashboardOnOpen() return true end +-- The refresh behind a dashboard that has just been shown. An explicit open +-- asks for a connection once, through the user's Wi-Fi policy, while the +-- cached dashboard is already on screen. Returning to the dashboard later +-- keeps the cache while offline rather than asking again. +function CatalogDashboard:refreshDashboardOnOpen() + if self:shouldRefreshDashboardOnOpen() then + self:loadDashboardRoot(true, { invisible = true }) + end + self.prefer_cached_dashboard = true +end + function CatalogDashboard:dashboardRoot(opts) local function requestIsCurrent() return not opts or not opts.is_current or opts.is_current() @@ -325,6 +336,11 @@ function CatalogDashboard:loadDashboardRoot(replace, opts) and self.current_context == expected_context end self:runConnected(function() + -- The screen said the dashboard could not be loaded while this waited + -- for a connection, so the fetch shows that it is now loading. + if expected_context and expected_context.unavailable then + request_opts.invisible = nil + end local items, context = self:dashboardRoot(request_opts) if self.catalog_closed or request_generation ~= self.dashboard_request_generation or self.current_context ~= expected_context then diff --git a/koreader-plugin/bookorbit.koplugin/main.lua b/koreader-plugin/bookorbit.koplugin/main.lua index 470bb71e7..756d8fdfd 100644 --- a/koreader-plugin/bookorbit.koplugin/main.lua +++ b/koreader-plugin/bookorbit.koplugin/main.lua @@ -1113,19 +1113,16 @@ function BookOrbit:openCatalogBrowser(prefer_cached_dashboard) UIManager:show(self.catalog_browser) end -function BookOrbit:browseCatalog(allow_offline) +-- Opening never waits for a connection: the catalog shows the cached dashboard +-- at once and asks for one itself when it refreshes. An offline-tolerant open +-- (the auto-open) does not ask at all. +function BookOrbit:browseCatalog(prefer_cached_dashboard) if not self:isLoggedIn() then promptLogin() return end - if allow_offline then - self:openCatalogBrowser(true) - else - NetworkMgr:runWhenConnected(function() - self:openCatalogBrowser(false) - end) - end + self:openCatalogBrowser(prefer_cached_dashboard == true) end -- Manual sync triggers diff --git a/koreader-plugin/spec/bookorbit_catalog_dashboard_startup_test.lua b/koreader-plugin/spec/bookorbit_catalog_dashboard_startup_test.lua index d27e847f0..4d360728a 100644 --- a/koreader-plugin/spec/bookorbit_catalog_dashboard_startup_test.lua +++ b/koreader-plugin/spec/bookorbit_catalog_dashboard_startup_test.lua @@ -192,4 +192,97 @@ assertEqual(last_fetch_opts.invisible, true, "automatic dashboard refresh is non assertEqual(switches, 0, "stale dashboard response cannot replace a navigated view") assertEqual(catalog.current_context, navigated_context, "navigation context remains current") +-- An explicit open while offline shows the cache at once and asks for a +-- connection a single time, through the user's Wi-Fi policy. Returning to the +-- dashboard while still offline keeps the cache rather than asking again. +local function openedCatalog(opts) + local opened = newCatalog(opts) + local _, initial = opened:initialDashboardContext() + opened.current_context = initial + opened.connection_requests = {} + function opened:runConnected(fn) + table.insert(self.connection_requests, fn) + end + function opened:switchTo(_, _, next_context) + self.current_context = next_context + end + return opened +end + +connected = false +catalog = openedCatalog{ cache = cached_dashboard } +assertEqual(catalog.current_context.dashboard, cached_dashboard, "an offline explicit open shows the cache at once") +catalog:refreshDashboardOnOpen() +assertEqual(#catalog.connection_requests, 1, "an offline explicit open asks for a connection") +catalog:refreshDashboardOnOpen() +assertEqual(#catalog.connection_requests, 1, "returning to the dashboard offline does not ask again") + +-- The connection arrives: the cached dashboard is refreshed in place, quietly. +connected = true +requests = 0 +catalog.connection_requests[1]() +assertEqual(requests, 1, "the refresh runs once the connection arrives") +assertEqual(last_fetch_opts.invisible, true, "a refresh behind the cached dashboard stays non-modal") +assertEqual(catalog.current_context.stale, false, "the cached dashboard is replaced by fresh data") +catalog:refreshDashboardOnOpen() +assertEqual(#catalog.connection_requests, 2, "returning to the dashboard online still refreshes") + +-- With no cache an offline explicit open says the dashboard could not be +-- loaded rather than promising a load that may never come, and still asks. +connected = false +catalog = openedCatalog{} +assertEqual(catalog.current_context.unavailable, true, "an offline open with no cache reports unavailable") +assertEqual(catalog.current_context.loading, false, "an offline open with no cache does not claim to be loading") +catalog:refreshDashboardOnOpen() +assertEqual(#catalog.connection_requests, 1, "an offline open with no cache still asks for a connection") +connected = true +catalog.connection_requests[1]() +assertEqual(last_fetch_opts.invisible, nil, "once connected, the fetch shows that the dashboard is loading") +assertEqual(catalog.current_context.unavailable, false, "the unavailable placeholder is replaced") + +-- The auto-open never asks for a connection, offline or on later returns. +connected = false +catalog = openedCatalog{ prefer_cached_dashboard = true, cache = cached_dashboard } +catalog:refreshDashboardOnOpen() +catalog:refreshDashboardOnOpen() +assertEqual(#catalog.connection_requests, 0, "an offline auto-open never asks for a connection") +connected = true + +-- Going home from deep in the catalog (the Dashboard button, hold Return) +-- switches to the cached dashboard at once instead of waiting on a connection. +-- bookorbit_catalog.lua cannot be loaded outside KOReader, so the method is +-- lifted out and run against the real dashboard methods. +local catalog_source = assert(io.open("koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua", "rb")) +local go_dashboard_source = assert(catalog_source:read("*a"):match("\n(function BookOrbitCatalog:goDashboard%(.-\nend)\n"), + "goDashboard is defined in bookorbit_catalog.lua") +catalog_source:close() +local next_ticks = {} +local go_dashboard_env = { + BookOrbitCatalog = {}, + UIManager = { + nextTick = function(_, fn) + table.insert(next_ticks, fn) + end, + }, +} +local go_dashboard_chunk = assert(loadstring(go_dashboard_source, "goDashboard")) +setfenv(go_dashboard_chunk, go_dashboard_env) +go_dashboard_chunk() + +connected = false +catalog = openedCatalog{ cache = cached_dashboard } +catalog:refreshDashboardOnOpen() +local detail_context = { kind = "detail" } +catalog.current_context = detail_context +catalog.stack = { { context = catalog.current_context } } +catalog.goDashboard = go_dashboard_env.BookOrbitCatalog.goDashboard +catalog.updateReturnPath = function() end +catalog:goDashboard() +assertEqual(catalog.current_context.kind, "dashboard", "going home offline shows the dashboard at once") +assertEqual(catalog.current_context.dashboard, cached_dashboard, "going home offline shows the cached dashboard") +assertEqual(#catalog.stack, 0, "going home clears the navigation stack") +assertEqual(#next_ticks, 1, "the refresh is queued behind the redraw") +next_ticks[1]() +assertEqual(#catalog.connection_requests, 1, "going home offline does not ask for a connection again") + print("bookorbit_catalog_dashboard_startup_test.lua: ok") diff --git a/koreader-plugin/spec/bookorbit_dashboard_offline_open_test.lua b/koreader-plugin/spec/bookorbit_dashboard_offline_open_test.lua new file mode 100644 index 000000000..6481d2ac1 --- /dev/null +++ b/koreader-plugin/spec/bookorbit_dashboard_offline_open_test.lua @@ -0,0 +1,132 @@ +-- Issue 1741: opening the dashboard must never wait for a connection. Every +-- entry point opens the catalog at once; only the auto-open is offline-tolerant +-- from the start, and asking for a connection is left to the catalog's refresh. + +package.loaded["gettext"] = function(text) + return text +end +package.loaded["ffi/util"] = { + template = function(text) + return text + end, +} +package.loaded["ui/uimanager"] = {} +package.loaded["device"] = { model = "test-device" } +package.loaded["ui/widget/infomessage"] = { new = function(_, opts) return opts end } +package.loaded["ui/widget/inputdialog"] = {} +package.loaded["ui/widget/multiinputdialog"] = {} +package.loaded["ui/widget/notification"] = {} +package.loaded["ffi/sha2"] = { md5 = function(value) return value end } +package.loaded["util"] = { + trim = function(value) + return tostring(value or ""):match("^%s*(.-)%s*$") + end, +} +package.loaded["bookorbit_api"] = {} +package.loaded["bookorbit_highlight_diagnostics"] = {} +package.loaded["bookorbit_sweep"] = {} + +local waits = 0 +local function waitForConnection() + waits = waits + 1 + return true +end +local NetworkMgr = { + isConnected = function() + return false + end, + runWhenConnected = waitForConnection, + willRerunWhenConnected = waitForConnection, + beforeWifiAction = waitForConnection, +} +package.loaded["ui/network/manager"] = NetworkMgr + +package.path = "koreader-plugin/bookorbit.koplugin/?.lua;" .. package.path + +local MainMenu = require("bookorbit_main_menu") + +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 read(path) + local file = assert(io.open(path, "rb")) + local content = file:read("*a") + file:close() + return content +end + +-- The entry points live in main.lua, which cannot be loaded whole outside +-- KOReader, so each one is lifted out and run against stubs. +local main = read("koreader-plugin/bookorbit.koplugin/main.lua") +local prompted_login = 0 +local env = { + BookOrbit = {}, + NetworkMgr = NetworkMgr, + promptLogin = function() + prompted_login = prompted_login + 1 + end, +} +for _, name in ipairs({ "isLoggedIn", "browseCatalog", "onBookOrbitOpenDashboard", "maybeAutoOpenCatalog" }) do + local source = assert(main:match("\n(function BookOrbit:" .. name .. "%(.-\nend)\n"), name .. " is defined in main.lua") + local chunk = assert(loadstring(source, name)) + setfenv(chunk, env) + chunk() +end + +local opened = {} +local plugin = { + SYNC_STRATEGY = { PROMPT = 1, SILENT = 2, DISABLE = 3 }, + settings = { server_url = "http://bookorbit.test/api/v1", username = "reader", userkey = "key" }, +} +MainMenu.install(plugin) +for name, fn in pairs(env.BookOrbit) do + plugin[name] = fn +end +plugin.settingsMenu = function() + return {} +end +plugin.openCatalogBrowser = function(_, prefer_cached_dashboard) + table.insert(opened, prefer_cached_dashboard) +end + +local function openDashboardMenuItem() + local menu_items = {} + plugin:addToMainMenu(menu_items) + for _, item in ipairs(menu_items.bookorbit.sub_item_table) do + if item.id == "open_dashboard" then return item end + end + error("the BookOrbit menu has an Open dashboard item") +end + +-- Tools > BookOrbit > Open dashboard while offline. +openDashboardMenuItem().callback() +assertEqual(#opened, 1, "the menu opens the dashboard while offline") +assertEqual(opened[1], false, "the menu open is explicit, so its refresh may ask for a connection") +assertEqual(waits, 0, "the menu does not wait for a connection before opening") + +-- The gesture and dispatcher action. +opened = {} +plugin:onBookOrbitOpenDashboard() +assertEqual(#opened, 1, "the dispatcher action opens the dashboard while offline") +assertEqual(opened[1], false, "the dispatcher action is an explicit open") +assertEqual(waits, 0, "the dispatcher action does not wait for a connection") + +-- The auto-open stays offline-tolerant: it never asks for a connection. +opened = {} +plugin.ui = { file_chooser = {} } +plugin:maybeAutoOpenCatalog("filemanager") +assertEqual(#opened, 1, "the auto-open opens the dashboard") +assertEqual(opened[1], true, "the auto-open prefers the cached dashboard") +assertEqual(waits, 0, "the auto-open does not wait for a connection") + +-- Logged out, nothing opens and the user is told to log in. +opened = {} +plugin.settings = {} +plugin:browseCatalog() +assertEqual(#opened, 0, "a logged-out open does not open the catalog") +assertEqual(prompted_login, 1, "a logged-out open asks the user to log in") + +print("bookorbit_dashboard_offline_open_test.lua: ok")